> -----Original Message-----
> From: Cédric Le Goater <[email protected]>
> Sent: Thursday, September 17, 2026 6:51 PM
> To: Manish Honap <[email protected]>; [email protected]; Ankit Agrawal
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Srirangan Madhavan
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Krishnakant Jaju <[email protected]>; Vikram Sethi <[email protected]>;
> Zhi Wang <[email protected]>; [email protected]; qemu-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 04/10] hw/vfio/pci: Enforce the passthrough topology
> for a CXL device
>
> External email: Use caution opening links or attachments
>
>
> On 9/16/26 20:44, [email protected] wrote:
> > From: Manish Honap <[email protected]>
> >
> > Only the guest's endpoint HDM decoder is programmed by the guest; the
> > decoders above it are covered only while the pxb-cxl host bridge stays
> > in passthrough mode. Reject a switch in the path, or a host bridge
> > taken out of passthrough, at realize before any mapping exists.
> >
> > AI-used-for: code (prototype)
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> > hw/vfio/pci.c | 73
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 73 insertions(+)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > 2f84af5cf8..10b13f200d 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -27,6 +27,7 @@
> > #include "hw/pci/msi.h"
> > #include "hw/pci/msix.h"
> > #include "hw/pci/pci_bridge.h"
> > +#include "hw/cxl/cxl.h"
> > #include "hw/core/qdev-properties.h"
> > #include "hw/core/qdev-properties-system.h"
> > #include "hw/vfio/vfio-cpr.h"
> > @@ -3570,6 +3571,73 @@ bool vfio_pci_interrupt_setup(VFIOPCIDevice
> *vdev, Error **errp)
> > return true;
> > }
> >
> > +/*
> > + * Walk the endpoint's ancestor bridges up to its pxb-cxl host
> > +bridge. A CXL
> > + * switch in the path is rejected: the guest would then have to
> > +program switch
> > + * decoders, which the passthrough model does not yet cover. This is
> > +the single
> > + * place the supported-topology assumption lives, so switch support
> > +later
> > + * relaxes it here.
> > + */
> > +static PXBCXLDev *vfio_cxl_find_pxb(VFIOPCIDevice *vdev, Error
> > +**errp) {
> > + PCIBus *bus = pci_get_bus(&vdev->parent_obj);
>
> Could this routine be moved to the CXL subsytem component ?
okay, this part reads pxb-cxl and CXL bus types that hw/cxl owns, so it fits
there. I am planning that move together with the CFMWS matching from patch 6,
so the vfio-pci side calls a small CXL API.
> > +
> > + if (!object_dynamic_cast(OBJECT(bus), TYPE_CXL_BUS)) {
> > + error_setg(errp,
> > + "vfio-cxl: %s: endpoint is not on a CXL root port
> > (cxl-rp)",
> > + vdev->vbasedev.name);
> > + return NULL;
> > + }
> > +
> > + for (; bus; ) {
>
> Why walk, since this is looking for the case "no-switch in between the bus and
> the ep" ? the no-switch case is just two hops in the topology.
Yes, the supported case is two hops EP <-> cxl-rp <-> pxb-cxl.
I will collapse the loop to this check during movement of the helper.
>
>
> > + PCIDevice *bridge = bus->parent_dev;
> > +
> > + if (!bridge) {
> > + break;
> > + }
> > + if (object_dynamic_cast(OBJECT(bridge), TYPE_CXL_USP) ||
> > + object_dynamic_cast(OBJECT(bridge), TYPE_CXL_DSP)) {
> > + error_setg(errp,
> > + "vfio-cxl: %s: switch-attached topology not
> > supported",
> > + vdev->vbasedev.name);
> > + return NULL;
> > + }
> > + if (object_dynamic_cast(OBJECT(bridge), TYPE_PXB_CXL_DEV)) {
> > + return PXB_CXL_DEV(bridge);
> > + }
> > + if (pci_bus_is_root(bus)) {
> > + break;
> > + }
> > + bus = pci_get_bus(bridge);
> > + }
> > +
> > + error_setg(errp, "vfio-cxl: %s: not attached below a pxb-cxl host
> > bridge",
> > + vdev->vbasedev.name);
> > + return NULL;
> > +}
> > +
> > +/*
> > + * Only guest's endpoint decoder is programmed, so the host bridge
> > +must stay in
> > + * HDM passthrough mode. Reject anything else at realize, before a
> > +mapping is
> > + * built. cxl_get_hb_passthrough() reflects reset state and is not
> > + * settled yet here, so gate on the static hdm_for_passthrough property.
> > + */
> > +static bool vfio_cxl_check_topology(VFIOPCIDevice *vdev, Error
> > +**errp) {
> > + PXBCXLDev *pxb = vfio_cxl_find_pxb(vdev, errp);
> > +
> > + if (!pxb) {
> > + return false;
> > + }
> > + if (pxb->hdm_for_passthrough) {
>
> This naming is a bit confusing:
>
> hdm_for_passthrough=false means "use HDM passthrough mode" (no
> decoders), while
> hdm_for_passthrough=true means "keep HDM decoders even in
> passthrough topology.
>
> I will get used to it.
Yes, I will add a comment at this point for more clarity.
>
> Thanks,
>
> C.
>
>
>
> > + error_setg(errp,
> > + "vfio-cxl: %s: the pxb-cxl must be in HDM passthrough
> > mode "
> > + "(do not set hdm_for_passthrough)",
> > vdev->vbasedev.name);
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > /*
> > * Learn the CXL geometry the kernel reports: the HPA-backed HDM memory
> region
> > * and the trapped HDM decoder block (which BAR carries it and at what
> offset).
> > @@ -3623,6 +3691,11 @@ static bool vfio_cxl_setup(VFIOPCIDevice *vdev,
> Error **errp)
> > cxl->dpa_size = mem_info->size;
> > cxl->comp_bar = cap->bar;
> > cxl->hdm_offset = cap->offset;
> > +
> > + if (!vfio_cxl_check_topology(vdev, errp)) {
> > + return false;
> > + }
> > +
> > cxl->enabled = true;
> >
> > return true;
