Re: [PATCH] xen/pt: Avoid initializing BARs from the host ones

[Ross Lagerwall]

> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: Monday, May 9, 2022 12:23 PM
> To: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>; Stefano 
> Stabellini <sstabellini@kernel.org>; Anthony Perard 
> <anthony.perard@citrix.com>; Paul Durrant <paul@xen.org>; 
> qemu-devel@nongnu.org <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] xen/pt: Avoid initializing BARs from the host ones 
>  
> On Mon, May 09, 2022 at 10:39:32AM +0000, Ross Lagerwall wrote:
> > > From: Roger Pau Monne <roger.pau@citrix.com>
> > > Sent: Monday, May 9, 2022 10:28 AM
> > > To: Ross Lagerwall <ross.lagerwall@citrix.com>
> > > Cc: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>; 
> > > Stefano Stabellini <sstabellini@kernel.org>; Anthony Perard 
> > > <anthony.perard@citrix.com>; Paul Durrant <paul@xen.org>; 
> > > qemu-devel@nongnu.org <qemu-devel@nongnu.org>
> > > Subject: Re: [PATCH] xen/pt: Avoid initializing BARs from the host ones 
> > >  
> > > On Wed, Apr 27, 2022 at 05:38:12PM +0100, Ross Lagerwall via wrote:
> > > > The BAR emulated register definition does not set emu_mask because it
> > > > varies depending on bar_flag.  If emu_mask is not set, then the BAR is
> > > > initialized based on the host value which causes the BAR to be initially
> > > > mapped at whatever value the host device was using. Although it does
> > > > eventually get mapped at the correct location, it causes unnecessary
> > > > mapping/unmappings.
> > > 
> > > Would it be possible to just unset the memory decoding bit in the
> > > command register if it's set?
> > 
> > I don't think that would be sufficient since AFAICT qemu does not read that
> > bit so will still call into Xen to update memory mappings, etc.
> 
> Hm, but this seems wrong?  QEMU should not set memory mappings if the
> memory decoding bit is unset.  While the bit will be set for the
> underlying physical device, it shouldn't be set in the emulated
> command register provided to the guest, and hence no mappings should
> be established until that bit is set by the guest.
> 
> In the description you mention not using the host BAR positions, which
> is fine, but you also need to prevent mappings from being created
> until the guest has positioned the BARs and enabled the memory
> decoding bit, or else you end up positioning the BARs wrongly has QEMU
> has no knowledge of where should BARs reside.
> 

I was wrong - I missed the point where QEMU checked the memory decoding bit
since it is in the core PCI code.

This patch was originally for some older combination of Xen/QEMU. I
rechecked now and I'm no longer able to reproduce the symptom in the
description (unnecessary incorrect mapping/unmappings) so I guess
something else changed in the meantime. The BAR register being
temporarily populated with host values before being programmed
is therefore not a big issue so I think this patch can just be
dropped.

Thanks,
Ross