On Thu, Feb 03, 2022 at 01:30:26PM +0000, Oleksandr Andrushchenko wrote:
>
>
> On 03.02.22 14:54, Jan Beulich wrote:
> > On 03.02.2022 13:45, Oleksandr Andrushchenko wrote:
> >>>> Also memory decoding needs to be initially disabled when used by
> >>>> guests, in order to prevent the BAR being placed on top of a RAM
> >>>> region. The guest physmap will be different from the host one, so it's
> >>>> possible for BARs to end up placed on top of RAM regions initially
> >>>> until the firmware or OS places them at a suitable address.
> >>> Agree, memory decoding must be disabled
> >> Isn't it already achieved by the toolstack resetting the PCI device
> >> while assigning it to a guest?
> > Iirc the tool stack would reset a device only after getting it back from
> > a DomU. When coming straight from Dom0 or DomIO, no reset would be
> > performed. Furthermore, (again iirc) there are cases where there's no
> > known (standard) way to reset a device. Assigning such to a guest when
> > it previously was owned by another one is risky (and hence needs an
> > admin knowing what they're doing), but may be acceptable in particular
> > when e.g. simply rebooting a guest.
> >
> > IOW - I don't think you can rely on the bit being in a particular state.
> So, you mean something like:
>
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 7695158e6445..9ebd57472da8 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -808,6 +808,14 @@ static int init_bars(struct pci_dev *pdev)
> return rc;
> }
>
> + /*
> + * Memory decoding needs to be initially disabled when used by
> + * guests, in order to prevent the BAR being placed on top of a RAM
> + * region.
> + */
> + if ( !is_hwdom )
> + pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd & ~PCI_COMMAND_MEMORY);
Memory decoding is already disabled here, so you just need to avoid
enabling it, for example:
/*
* Memory decoding needs to be initially disabled when used by
* guests, in order to prevent the BARs being mapped at gfn 0 by
* default.
*/
if ( !is_hwdom )
cmd &= ~PCI_COMMAND_MEMORY;
> return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
This is important here because guest_reg won't be set (ie: will be set
to 0) so if for some reason memory decoding was enabled you would end
up with BARs mappings overlapping at gfn 0.
Thanks, Roger.
