Re: [RFC 7/9] hw/vfio+cxl: Program HDM decoder 0 at machine_done for firmware-committed devices

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 27 Apr 2026 23:42:33 +0530
<mhonap@nvidia.com> wrote:

> From: Manish Honap <mhonap@nvidia.com>
> 
> setup_locked_hdm() runs as a machine_done notifier after all devices
> have been realized. It programs HDM decoder 0 with the CFMWS base

Be more specific on this.  Which HDM Decoder?  i.e. what device
is it in.  Ultimately there may be several in the path.  Are you
supporting that?  The reference to FLR() later would only apply to
an endpoint (can't FLR a host bridge) + the code is clearly operating
on the shadow registers for the EP.

I guess maybe you are relying on pass through host bridge decoders?
If you are - you need to go check those - or better yet program
and lock them as well.

> address so the guest can fault into device memory from the first
> instruction.
> 
> The notifier is only registered when the kernel reports the device as
> firmware-committed (VFIO_CXL_CAP_FIRMWARE_COMMITTED). The host is
> responsible for HDM decoder programming; the guest has no mechanism to
> remap host physical address mappings.

Maybe say why this matters.  Presumably because only thing we can
program from the guest is GPA to HPA mappings so linear ranges remain
linear etc (more or less anyway given constraints on how many
regions we can plug into qemu!)

> 
> The function uses cxl->fmws_base (set by the optional cxl-fmws-base
> device property) if non-zero; otherwise it falls back to the

Why is that property needed?  If it is add it in a separate patch where
you can explain the need to force it. Smells like a hack so far.

> cxl_fmws_base global captured by cxl_fmws_set_memmap() during machine
> memory-map init. If neither is set, it warns and returns without
> programming anything.

Which CFMWS is that base in?  How is that controlled?  We may well
have some type 3 devices in same VM and probably want those in different
CFMWs (we'll need to actually upstream the patch to control constraints
so the kernel won't try using your CFMWS for those - currently all CFMWS
are entirely flexible).


> 
> If COMMIT_LOCK is set in decoder 0 CTRL at machine_done time (left-over
> from a prior FLR?), it is cleared before writing BASE so the subsequent
> write is not blocked. COMMIT_LOCK is re-set after programming so the
> hardware enforces the committed base.

Hmm. We should look into whether there is a correct way to reset that.
Though as above from the description I'm not sure which decoder this
is resetting.

> 
> read_region() return is checked; failure aborts programming rather than
> leaving ctrl uninitialized. All write_region() failures are propagated.
> The function exits cleanly rather than leaving the decoder half-programmed.
> 
> Add cxl_fmws_base as a hwaddr global in cxl-host.c (and a stub in
> cxl-host-stubs.c). It is set once by cxl_fmws_set_memmap() and read
> later at machine_done time.
> 
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Manish Honap <mhonap@nvidia.com>


> @@ -3486,6 +3646,18 @@ static bool vfio_cxl_setup(VFIOPCIDevice *vdev, Error **errp)
>      trace_vfio_cxl_setup_params(vbasedev->name, cxl->hdm_regs_bar_index,
>                                   cxl->hdm_regs_offset, cxl->hdm_regs_size,
>                                   cxl->dpa_size);
> +
> +    /*
> +     * Only pre-program the HDM decoder if the kernel reported the device as
> +     * firmware-committed.  Non-committed devices need guest driver involvement

For wording these can we be very clear which kernel.  Though I'm also unsure
what FIRMWARE_COMMITTED means here.  Does it actually mean host firmware, or does
it mean something running on the host pre guest boot (e.g. could it be qemu?)

> +     * to commit the decoder; registering the notifier for them would write an
> +     * uncommitted BASE value that the hardware ignores.
> +     */
> +    if (cap->flags & VFIO_CXL_CAP_FIRMWARE_COMMITTED) {
> +        cxl->machine_done.notify = setup_locked_hdm;
> +        qemu_add_machine_init_done_notifier(&cxl->machine_done);
> +    }
> +
>      return true;
>  }
>