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

[Matthew Rosato]

On 12/11/24 9:40 AM, Cédric Le Goater wrote:
> On 12/9/24 20:29, Matthew Rosato wrote:

>>   static int reg_ioat(CPUS390XState *env, S390PCIBusDevice *pbdev, ZpciFib 
>> fib,
>>                       uintptr_t ra)
>>   {
>> @@ -1008,7 +1036,7 @@ 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)) {
> 
> Is this change part of the patchset ? It looks like some other issue.
> I might be wrong.
> 

Definitely part of this patch.
Before we only allow a single type of translation request from the guest.  So, 
guest needed to be requesting translation (t) and it had to be of a certain 
type (dt==ZPCI_IOTA_RTTO) else we fail.
Now we are allowing either that same single translation type (so t && 
dt==ZPCI_IOTA_RTTO) OR no translation (!t).  So it becomes valid to have (!t) 
but still invalid to have (t) with any other dt value besides 
(dt==ZPCI_IOTA_RTTO), hence the new check.

>>           error_report("unsupported ioat dt %d t %d", dt, t);
>>           s390_program_interrupt(env, PGM_OPERAND, ra);
>>           return -EINVAL;
>> @@ -1018,13 +1046,23 @@ static int reg_ioat(CPUS390XState *env, 
>> S390PCIBusDevice *pbdev, ZpciFib fib,
>>       iommu->pal = pal;
>>       iommu->g_iota = g_iota;
>>   -    s390_pci_iommu_enable(iommu);
>> +    if (t) {
>> +        s390_pci_iommu_enable(iommu);
>> +    } else {
>> +        s390_pci_iommu_dm_enable(iommu);
>> +        /* If not translating, map everything now */
>> +        s390_pci_setup_stage2_map(iommu);
>> +    }
> 
> 
> I don't understand how we can enter "map everything" case.
> Could you explain a bit more the scenario ?
> 

Sure, this actually relates directly to the check I discussed up above...

Before, we would only ever accept a guest MPCIFC instruction of type 
ZPCI_MOD_FC_REG_IOAT that specified "designation type 1 with translation".  All 
else would be rejected.
Now, we would accept either the above OR a guest MPCIFC instruction that 
specifies "no translation" - this is the case that gets us into the "map 
everything" scenario.

Patch 2 adds a CLP indication that we advertise to the guest that tells them 
whether or not the device group in question supports the "map everything case", 
so it's safe/allowable to issue a MPCIFC instruction that specifies "no 
translation" to a device in that group.

The referenced kernel series in the cover letter implements the linux guest 
behavior necessary to exploit this "no translation" case via the optional 
kernel parameter iommu.passthrough=1.  

Hope that helps,
Matt