On Fri, May 08, 2020 at 02:43:38PM +0200, Jan Beulich wrote:
> The op has a register/unregister flag, and hence registration shouldn't
> happen unilaterally. Introduce unregister_vpci_mmcfg_handler() to handle
> this case.
>
> Fixes: eb3dd90e4089 ("x86/physdev: enable PHYSDEVOP_pci_mmcfg_reserved for
> PVH Dom0")
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -558,6 +558,47 @@ int register_vpci_mmcfg_handler(struct d
> return 0;
> }
>
> +int unregister_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
> + unsigned int start_bus, unsigned int
> end_bus,
> + unsigned int seg)
> +{
> + struct hvm_mmcfg *mmcfg;
> + int rc = -ENOENT;
> +
> + ASSERT(is_hardware_domain(d));
> +
> + if ( start_bus > end_bus )
> + return -EINVAL;
> +
> + write_lock(&d->arch.hvm.mmcfg_lock);
> +
> + list_for_each_entry ( mmcfg, &d->arch.hvm.mmcfg_regions, next )
> + if ( mmcfg->addr == addr + (start_bus << 20) &&
> + mmcfg->segment == seg &&
> + mmcfg->start_bus == start_bus &&
> + mmcfg->size == ((end_bus - start_bus + 1) << 20) )
> + {
> + list_del(&mmcfg->next);
> + if ( !list_empty(&d->arch.hvm.mmcfg_regions) )
> + xfree(mmcfg);
> + else
> + {
> + /*
> + * Cannot unregister the MMIO handler - leave a fake entry
> + * on the list.
> + */
> + memset(mmcfg, 0, sizeof(*mmcfg));
> + list_add(&mmcfg->next, &d->arch.hvm.mmcfg_regions);
Instead of leaving this zombie entry around maybe we could add a
static bool in register_vpci_mmcfg_handler to signal whether the MMIO
intercept has been registered?
> + }
> + rc = 0;
> + break;
> + }
> +
> + write_unlock(&d->arch.hvm.mmcfg_lock);
> +
> + return rc;
> +}
> +
> void destroy_vpci_mmcfg(struct domain *d)
> {
> struct list_head *mmcfg_regions = &d->arch.hvm.mmcfg_regions;
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -559,12 +559,18 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
> if ( !ret && has_vpci(currd) )
> {
> /*
> - * For HVM (PVH) domains try to add the newly found MMCFG to the
> - * domain.
> + * For HVM (PVH) domains try to add/remove the reported MMCFG
> + * to/from the domain.
> */
> - ret = register_vpci_mmcfg_handler(currd, info.address,
> - info.start_bus, info.end_bus,
> - info.segment);
> + if ( info.flags & XEN_PCI_MMCFG_RESERVED )
Do you think you could also add a small note in physdev.h regarding
the fact that XEN_PCI_MMCFG_RESERVED is used to register a MMCFG
region, and not setting it would imply an unregister request?
It's not obvious to me from the name of the flag.
Thanks, Roger.
