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

[Thomas Huth]

On 27/01/2025 21.45, Matthew Rosato wrote:
> > >    #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.

The qemu_log_mask() stuff has to be enabled on the command line with "-d 
guest_errors".

>  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!

Ok, fine for me, too.

 Thomas