Re: [PATCH v7 2/2] hw/acpi: Implement the SRAT GI affinity structure

[Jonathan Cameron]

On Tue, 27 Feb 2024 17:11:15 +0000
Jonathan Cameron via <qemu-devel@nongnu.org> wrote:

> On Tue, 27 Feb 2024 08:37:15 +0000
> Ankit Agrawal <ankita@nvidia.com> wrote:
> 
> > Thanks Jonathan for reviewing the change.
> > 
> > Comments inline.
> >   
> > >> The structure needs a PCI device handle [2] that consists of the device 
> > >> BDF.
> > >> The vfio-pci device corresponding to the acpi-generic-initiator object is
> > >> located to determine the BDF.
> > >>
> > >> [1] ACPI Spec 6.3, Section 5.2.16.6
> > >> [2] ACPI Spec 6.3, Table 5.80
> > >>
> > >> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>    
> > >Hi Ankit,
> > >
> > > As the code stands the use of a list seems overkill.    
> > 
> > Yeah, I will try out your suggestion.
> >   
> > > Otherwise looks good to me.  I need Generic Ports support for CXL
> > > stuff so will copy your approach for that as it's ended up nice
> > > and simple.
> > > 
> > > Jonathan    
> > 
> > Nice, would be good to have uniform implementations.  
> 
> I've been messing around with this today.
> 
> They differ only very trivially.
> 2 Options.
> 1) Have acpi-generic-port inherit from acpi-generic-initiator.
>    Works but implies a relationship that isn't really true.
> 2) Add an abstract base class. I've called it acpi-generic-node
>    and have bother acpi-generic-initiator and acpi-generic-port
>    inherit from it.
> 
> The second feels more natural but is a tiny bit more code (a few
> more empty init / finalize functions.
> 
> If we are going to end up with an abstract base 'object' it
> will be cleaner to do this all as one series if you don't mind
> carrying the generic port stuff as well? Or I can smash the
> two series together and send out an updated version that hopefully
> meets both our requirements (+ tests etc).
> 
> I'm just running tests against the CXL qos / generic port code
> but assuming all goes well can share my additional changes
> in next day or two.
> 
> Jonathan

One more thing.  Right now we can't use Generic Initiators as
HMAT initiators.  That also wants fixing given that's their
normal usecase rather than what you are using them for so it
should 'work'.

Jonathan

> 
> 
>