Re: [PATCH 0/2] hw/pci-bridge: pci_expander_bridge: Fix wrong type and rework inheritance.

[Michael S. Tsirkin]

On Fri, Apr 21, 2023 at 01:19:08PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Apr 2023 09:59:57 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> > On Fri, 21 Apr 2023 at 09:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Thu, Apr 20, 2023 at 03:27:48PM +0100, Jonathan Cameron wrote:  
> > > > Peter Maydell highlighted an incorrect conversion to TYPE_PXB_DEVICE 
> > > > from
> > > > a device that didn't have that a an ancestor type. PXB_DEV() used 
> > > > instead of
> > > > PXB_CXL_DEV()/
> > > >
> > > > CAFEAcA-+de+eeLCE4YsAw1O-Qyd_4W1Ra05mGDsU_-3a6d92qw@mail.gmail.com/">https://lore.kernel.org/qemu-devel/CAFEAcA-+de+eeLCE4YsAw1O-Qyd_4W1Ra05mGDsU_-3a6d92qw@mail.gmail.com/
> > > >
> > > > During the discussion it became clear that the inheritance of the 
> > > > various
> > > > TYPE_PXB*_DEVICE was unusual. This patchset first provides the minimal
> > > > fix then cleans up the inheritance of types based on functionality.
> > > >
> > > > There is also a rename to TYPE_PXB*_DEV to allow removal of some 
> > > > boilerplate.
> > > >
> > > > Before this series
> > > > TYPE_PXB_DEVICE, TYPE_PXB_PCIE_DEVICE and TYPE_PXB_CXL_DEVICE all
> > > > had TYPE_PCI_DEVICE as their direct parent though they shared a common
> > > > struct PXBDev for their state.  As a result this state contained
> > > > some data that was irrelevant for some the types.
> > > >
> > > > This series changes to
> > > > TYPE_PXB_CXL_DEV has a parent of TYPE_PXB_PCIE_DEV
> > > > TYPE_PXB_PCIE_DEV has a parent of TYPE_PXB_DEV
> > > > TYPE_PXB_DEV continues to have a parent of TYPE_PCI_DEVICE.
> > > >
> > > > Each of the TYPE_PXB*_DEV has a state structure adding those elements
> > > > to their parent that they need. This also allowed dropping a wrapping
> > > > structure for the CXL state as the PXBCXLDev structure already provides
> > > > the equivalent grouping.
> > > >
> > > > Patches are similar to those posted in the thread but rebased on 
> > > > v8.0.0.  
> > >
> > > this conflicts with
> > >     Revert "hw/pxb-cxl: Support passthrough HDM Decoders unless 
> > > overridden"
> > >
> > > I think you acked that one?  
> > 
> > We should take one or the other, but not both. If this patchset
> > is good then it's probably better to fix the bug rather than
> > revert the feature, I think.
> 
> If it's easy to drop the revert that would be my preference.

OK, I'll do that. Thanks!

> If not, then I'm fine spinning a new version of that patch without
> the bug (so with patch 1 of this squashed in).  Patch 2 is somewhat related
> refactoring.  Not necessary to fix the issue even though it was motivated
> by that bug.
> 
> Jonathan
> 
> > 
> > -- PMM