Re: [PATCH] xen/vpci: allow BAR write if value is the same

From: Jan Beulich <jbeulich@suse.com>
To: Stewart Hildebrand <stewart.hildebrand@amd.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, xen-devel@lists.xenproject.org
Subject: Re: [PATCH] xen/vpci: allow BAR write if value is the same
Date: Tue, 24 Oct 2023 09:09:45 +0200	[thread overview]
Message-ID: <2ef80dad-5ae6-786d-e89f-d0c3af44b485@suse.com> (raw)
In-Reply-To: <20231023163615.693462-1-stewart.hildebrand@amd.com>

On 23.10.2023 18:36, Stewart Hildebrand wrote:
> During xl pci-assignable-add, pciback may reset the device while memory decoding
> (PCI_COMMAND_MEMORY) is enabled. After device reset, memory decoding may be
> disabled in hardware, and the BARs may be zeroed/reset in hardware. However, Xen
> vPCI still thinks memory decoding is enabled, and BARs will remain mapped in
> p2m. In other words, memory decoding may become disabled and BARs reset in
> hardware, bypassing the respective vPCI command and BAR register handlers.
> Subsequently, when pciback attempts to restore state to the device, including
> BARs, it happens to write the BARs before writing the command register.
> Restoring/writing the BARs silently fails because Xen vPCI mistakenly thinks
> memory decoding is enabled.
> 
> Fix the BAR write by allowing it to succeed if the value written is the same as
> the Xen vPCI stored value. pciback will subsequently restore the command
> register and the state of the BARs and memory decoding bit will then be in sync
> between hardware and vPCI again.
> 
> While here, remove a nearby stray newline.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
> Do we need similar handling in rom_write()?

I think so, if we are to go this route. However, ...

> We may consider additionally checking during bar_write() if the memory decoding
> state has become out of sync between hardware and vPCI. During bar_write(), we
> would check the device's memory decoding bit, compare it to our vPCI state, and
> invoke modify_bars() if needed. Please let me know your thoughts.

... iirc we discussed earlier on that doing resets in Dom0 wants
communicating to Xen. Any way of resetting by a DomU would likely need
intercepting. This way the described situation can be avoided altogether.
We may go further and uniformly intercept resets, carrying them out on
behalf of Dom0 as well. The main issue is, as with any config-space-
based interaction with a device, that there may be device specific ways
of resetting.

Jan