Re: [Xen-devel] [PATCH v4 3/5] vpci/msix: carve p2m hole for MSIX MMIO regions

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Nov 19, 2018 at 07:56:18AM -0700, Jan Beulich wrote:
> >>> On 14.11.18 at 12:57, <roger.pau@xxxxxxxxxx> wrote:
> > Make sure the MSIX MMIO regions don't have p2m entries setup, so that
> > accesses to them trap into the hypervisor and can be handled by vpci.
> > 
> > Commit 042678762 ("x86/iommu: add map-reserved dom0-iommu option to
> > map reserved memory ranges") added mappings for all the reserved
> > regions into the PVH Dom0 p2m, and some of those reserved regions
> > might contain MSIX MMIO regions, hence the need to make sure there are
> > no mappings established.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> despite ...
> 
> > --- a/xen/drivers/vpci/header.c
> > +++ b/xen/drivers/vpci/header.c
> > @@ -89,6 +89,17 @@ static void modify_decoding(const struct pci_dev *pdev, 
> > uint16_t cmd,
> >      bool map = cmd & PCI_COMMAND_MEMORY;
> >      unsigned int i;
> >  
> > +    /*
> > +     * Make sure there are no mappings in the MSIX MMIO areas, so that 
> > accesses
> > +     * can be trapped (and emulated) by Xen when the memory decoding bit is
> > +     * enabled.
> > +     *
> > +     * FIXME: punching holes after the p2m has been set up might be racy 
> > for
> > +     * DomU usage, needs to be revisited.
> > +     */
> > +    if ( map && !rom_only && vpci_make_msix_hole(pdev) )
> > +        return;
> 
> ... this still worrying me, as I think it'll need to be re-implemented
> altogether at that later point.

I expect this whole BAR mapping and unmapping to be limited to Dom0,
DomU should have a much more static physmap so that it can be setup by
the toolstack at creation time and the guest should not be allowed to
modify it.

That being set I also don't specially like this code, it's too
complex and requires iteration over all the devices for every
map/unmap operation. I have plans to improve this, which should be
discussed in a different email, but that's post-4.12.

> Afaict this patch is independent of the earlier two (leaving aside
> some fuzz that would be easy to resolve), which means it could
> go in ahead of them.

I plan to resend the series quite soon with the other comments fixed,
so let me know if I should drop this patch, I will also keep an eye on
the repo.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel