RE: [PATCH 07/10] hw/vfio/pci: Map the CXL memory on the guest decoder commit

> -----Original Message-----
> From: Junjie Cao <[email protected]>
> Sent: 18 August 2026 15:23
> To: Manish Honap <[email protected]>
> Cc: [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];
> [email protected]; Krishnakant Jaju <[email protected]>; Vikram Sethi
> <[email protected]>; Zhi Wang <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH 07/10] hw/vfio/pci: Map the CXL memory on the guest
> decoder commit
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Manish,
> 
> On Thu, 13 Aug 2026 18:36:20 +0530, Manish Honap wrote:
> > +    switch (le32_to_cpu(cap) & 0xf) {
> > +    case 0:  return 1;
> > +    case 1:  return 2;
> > +    case 2:  return 4;
> > +    case 3:  return 6;
> > +    case 4:  return 8;
> > +    default: return 1;
> 
> The encoding continues: 5h = 10 is the most a device may advertise, and
> switch/host-bridge encodings go on to Ch = 32 (CXL r3.1 8.2.4.20.1), so
> a 10-decoder endpoint takes the fallback and "walk every decoder"
> becomes "check decoder 0 only". Unreachable while the kernel gates on a
> single decoder. cxl_decoder_count_dec() in hw/cxl/cxl-component-utils.c
> already carries the full table and this file already includes its
> header; reusing it needs a floor of 1 (it decodes reserved encodings to
> 0) plus the stub treatment patch 6 gives cxl_get_hb_passthrough().

Agreed, there is no reason to keep a partial table. I will use
cxl_decoder_count_dec() with a floor of 1 for the reserved-to-0 case, and
add the CONFIG_CXL stub the way patch 6 does for cxl_get_hb_passthrough().

> 
> > + * GPA and never sees the host physical base the kernel shadow holds;
> > + the shadow
> > + * base and size are not consulted here.
> 
> The cover has the guest driving its own virtual decoder, and this patch
> handles a decommit. After a decommit, what refuses a re-commit with a
> base elsewhere inside an oversized window (patch 6 only warns on size >
> need)? The kernel FSM runs on the written shadow, QEMU maps at the
> window base regardless, and the virtualized read-back also reports the
> window base, so a divergence is silent -- guest accesses at the base it
> programmed land in the CFMWS trap instead of the device memory. If the
> FSM rejects any base other than the firmware value, a comment here
> closes the question; otherwise the write trap already sees every base
> write, so checking it against fmws_base on commit is cheap.

Good point. The guest is only ever expected to program the base it reads
back, but nothing is enforcing that...
I will check the written base against fmws_base on commit and
refuse the map on a mismatch, rather than rely on the kernel FSM implicitly.

> 
> >  static void vfio_cxl_teardown(VFIOPCIDevice *vdev)
> 
> Nothing on the unrealize path drops the mapping: the teardown runs at
> instance finalize, and the mapping itself blocks finalize, since adding
> the region into system memory referenced its owner, the vdev, and only
> the unmap drops that reference. An unplug flow that resets the device
> escapes through vfio_pci_pre_reset()'s PCI_COMMAND clear; ACPI hotplug
> eject unparents with no reset, so the vdev never finalizes, the VFIO fd
> stays held, and the guest keeps reading the removed device's HDM at the
> window base. Unmapping from vfio_exitfn() with the rest of the
> unrealize-time teardown breaks the cycle.

okay, I will drop the mapping from vfio_exitfn() at unrealize time.

> 
> Two notes from reading, no action needed:
> 
> The BASE_LOW[31:28]-only substitution is exact: cxl-fmw sizes are
> validated as 256MiB multiples (hw/cxl/cxl-host.c) and both machines hand
> cxl_fmws_set_memmap() a 256MiB-aligned start (hw/i386/pc.c,
> hw/arm/virt.c), so fw->base cannot carry bits below 28.
> 
> Migration is blocked today by the auto-mode blocker your patch 3 comment
> describes, but the destination path already holds for the day that
> lifts: vfio_pci_load_config() pushes PCI_COMMAND through
> vfio_pci_write_config(), which re-runs the commit scan after the
> machine-done notifier has bound the window.
> 
> Many thanks,
> Junjie