Re: [PATCH v10 12/17] vpci/header: emulate PCI_COMMAND register for guests

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Roger, Stewart,

Roger Pau Monné <roger.pau@citrix.com> writes:

> On Thu, Oct 12, 2023 at 10:09:18PM +0000, Volodymyr Babchuk wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> 
>> Xen and/or Dom0 may have put values in PCI_COMMAND which they expect
>> to remain unaltered. PCI_COMMAND_SERR bit is a good example: while the
>> guest's view of this will want to be zero initially, the host having set
>> it to 1 may not easily be overwritten with 0, or else we'd effectively
>> imply giving the guest control of the bit. Thus, PCI_COMMAND register needs
>> proper emulation in order to honor host's settings.
>> 
>> According to "PCI LOCAL BUS SPECIFICATION, REV. 3.0", section "6.2.2
>> Device Control" the reset state of the command register is typically 0,
>> so when assigning a PCI device use 0 as the initial state for the guest's view
>> of the command register.
>> 
>> Here is the full list of command register bits with notes about
>> emulation, along with QEMU behavior in the same situation:
>> 
>> PCI_COMMAND_IO - QEMU does not allow a guest to change value of this bit
>> in real device. Instead it is always set to 1. A guest can write to this
>> register, but writes are ignored.
>> 
>> PCI_COMMAND_MEMORY - QEMU behaves exactly as with PCI_COMMAND_IO. In
>> Xen case, we handle writes to this bit by mapping/unmapping BAR
>> regions. For devices assigned to DomUs, memory decoding will be
>> disabled and the initialization.
>> 
>> PCI_COMMAND_MASTER - Allow guest to control it. QEMU passes through
>> writes to this bit.
>> 
>> PCI_COMMAND_SPECIAL - Guest can generate special cycles only if it has
>> access to host bridge that supports software generation of special
>> cycles. In our case guest has no access to host bridges at all. Value
>> after reset is 0. QEMU passes through writes of this bit, we will do
>> the same.
>> 
>> PCI_COMMAND_INVALIDATE - Allows "Memory Write and Invalidate" commands
>> to be generated. It requires additional configuration via Cacheline
>> Size register. We are not emulating this register right now and we
>> can't expect guest to properly configure it. QEMU "emulates" access to
>> Cachline Size register by ignoring all writes to it. QEMU passes through
>> writes of PCI_COMMAND_INVALIDATE bit, we will do the same.
>> 
>> PCI_COMMAND_VGA_PALETTE - Enable VGA palette snooping. QEMU passes
>> through writes of this bit, we will do the same.
>> 
>> PCI_COMMAND_PARITY - Controls how device response to parity
>> errors. QEMU ignores writes to this bit, we will do the same.
>> 
>> PCI_COMMAND_WAIT - Reserved. Should be 0, but QEMU passes
>> through writes of this bit, so we will do the same.
>> 
>> PCI_COMMAND_SERR - Controls if device can assert SERR. QEMU ignores
>> writes to this bit, we will do the same.
>> 
>> PCI_COMMAND_FAST_BACK - Optional bit that allows fast back-to-back
>> transactions. It is configured by firmware, so we don't want guest to
>> control it. QEMU ignores writes to this bit, we will do the same.
>> 
>> PCI_COMMAND_INTX_DISABLE - Disables INTx signals. If MSI(X) is
>> enabled, device is prohibited from asserting INTx as per
>> specification. Value after reset is 0. In QEMU case, it checks of INTx
>> was mapped for a device. If it is not, then guest can't control
>> PCI_COMMAND_INTX_DISABLE bit. In our case, we prohibit a guest to
>> change value of this bit if MSI(X) is enabled.
>
> FWIW, bits 11-15 are RsvdP, so we will need to add support for them
> after the series from Stewart that adds support for register masks is
> accepted.

Stewart's series implement much better register handling than this
patch. What about dropping this change at all in favor of Stewart's
implementation? I'll be 100% okay with this.

[...]

-- 
WBR, Volodymyr