Re: [PATCH v3 1/2] s390x/pci: add support for guests that request direct mapping

[Matthew Rosato]

>>   #include "hw/s390x/s390-pci-bus.h"
>> @@ -1008,17 +1009,25 @@ static int reg_ioat(CPUS390XState *env, 
>> S390PCIBusDevice *pbdev, ZpciFib fib,
>>       }
>>         /* currently we only support designation type 1 with translation */
>> -    if (!(dt == ZPCI_IOTA_RTTO && t)) {
>> +    if (t && !(dt == ZPCI_IOTA_RTTO)) {
> 
> While you're at it, you could change that "!(dt == ZPCI_IOTA_RTTO)" into
> "dt != ZPCI_IOTA_RTTO".
> 

ACK

>>           error_report("unsupported ioat dt %d t %d", dt, t);
>>           s390_program_interrupt(env, PGM_OPERAND, ra);
>>           return -EINVAL;
>> +    } else if (!t && !pbdev->rtr_allowed) {
>> +        error_report("relaxed translation not allowed");
> 
> Not sure, but maybe better use qemu_log_mask(LOG_GUEST_ERROR, ...) instead?
> 

Hrm, this one I'm not so sure.  If I force this path and we give back the 
operand exception to the guest, the guest kernel is going to panic as a result. 
 We are very much in a "that wasn't ever supposed to happen" path, just like 
the unsupported dt above, because we already reported what we do (not) support 
via CLP.

I just tried to force the path with default settings + the above change, and I 
don't see anything in the qemu log from qemu_log_mask(LOG_GUEST_ERROR, ...) but 
I do with error_report.  Given the unlikely scenario that we reach this path 
and the potential ramifications, I think I'd rather leave it as error_report 
unless you've got a strong opinion on it!

Thanks,
Matt