[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/pt: Avoid initializing BARs from the host ones
From: |
Ross Lagerwall |
Subject: |
Re: [PATCH] xen/pt: Avoid initializing BARs from the host ones |
Date: |
Mon, 9 May 2022 10:39:32 +0000 |
> 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.
>
> > To fix this, initialize a per-register emu_mask in XenPTReg from the
> > initial value in XenPTRegInfo and then let the register's init() function
> > set/modify the emu_mask if necessary. Update the code to use emu_mask
> > in XenPTReg consistently and rename the existing emu_mask in
> > XenPTRegInfo to emu_mask_init to help with refactoring.
>
> Iff we really need this refactoring it would better be done in a
> separate (pre)patch, so it's clear what's the fix and what are
> refactoring changes.
>
> I also wonder whether why it isn't enough to set emu_mask in
> xen_pt_bar_reg_init(), without having to introduce a new field.
>
That doesn't work since emu_mask is part of xen_pt_emu_reg_header0 which
is static and is not defined per passthrough device so having multiple
passthrough devices would cause issues.
I have an idea to achieve the same result with less code churn so I'll
try it and send an updated patch.
Ross