Re: [PATCH 0/8] s390x/pci: Fixing s390 vfio-pci ISM support

[Pierre Morel]

On 1/21/21 3:42 PM, Matthew Rosato wrote:
> On 1/21/21 3:27 AM, Pierre Morel wrote:
> > On 1/20/21 9:29 PM, Matthew Rosato wrote:
> > > On 1/20/21 2:18 PM, Pierre Morel wrote:
> > > >
> > ...snip...
> >
> > > > So we have:
> > > > devices supporting MIO and MSIX
> > > > devices not supporting MIO nor MSIX
> > > > devices not supporting the use of PCISTG to emulate PCISTB
> > > >
> > > > The first two are two different things indicated by two different 
> > > > entries in the clp query PCI function response.
> > > >
> > > > The last one, we do not have an indicator as if the relaxed 
> > > > alignment and length is set, PCISTB can not be emulated with PCISTG
> >
> >
> > hum sorry, it seems I rewrote my sentence until it was wrong wrong!
> > I wanted to say we DO HAVE an indicator with the relaxed bit...
>
> That's actually not quite true though...  The relaxed bit does not 
> directly imply that PCISTB cannot be emulated with PCISTG.


It does, PCISTG have stronger alignment enforcement as PCISTB with the 
relaxed bit set, as you say here under, so we agree.


>   Rather, it 
> more generally implies that PCISTB could be used instead of PCISTG as 
> the length and alignment requirements for PCISTB are waived.  As far as 
> I can tell, disallowing a PCISTG altogether is a trait that is unique to 
> ISM and I don't see anywhere that it's otherwise indicated in 
> architecture...  And in fact, for a given ISM device, certain address 
> spaces (command) WILL accept a series of PCISTG issued in a particular 
> manner in place of a PCISTB; meanwhile other ISM address spaces (data) 
> will not accept any PCISTG ever.  :(  The ISM driver just always uses 
> PCISTB.

Very interresting!

> So that is why I suggested type==ISM must require use of the region 
> whereas other types that implement MSI could request (but not require) 
> use of the region.
>
> But yes, any other theoretical piece of hardware that also does not 
> support MIO would have a similar region requirement.  I'll have a look 
> at the MIO bit you referenced below and will have to verify that it does 
> exactly what we expect for an ISM device.  Assuming yes, I will consider 
> the following sort of checking for the next version of QEMU...
>
> if (!MIO) {
>      if (MSIX) {
>          // passthrough disallowed
>      }
>      else {
>          // region required, disallow if unavailable
>      }
> }
> else if(RELAX && !MSIX) {
>      // use region if available
> }
>
> Sound good?

Yes sounds good to me.
I mean the idea, I will not try to simplify, I trust you on this.


Thanks,

Pierre

--
Pierre Morel
IBM Lab Boeblingen