Deferring the actual write of the PCI register bit, either the memory
decoding or the ROM enable is not helpful, and adds an unnecessary amount of
complexity to the preemptible handling of BAR related p2m modifications.
In the hardware domain case, whether the PCI register write is done ahead
or after the p2m changes doesn't matter, a hardware domain has plenty of
ways to mess with the PCI register state if it wants to. Any poking at the
BAR p2m regions ahead of the guest write having completed will be
undefined.
On the other hand, for domUs the memory decoding bit shouldn't really
change as a result of guest actions, and should always be enabled. Guest
toggling the memory decoding command register should only result in p2m
modifications, but no propagation to the device PCI registers. Having
memory decoding unconditionally enabled ensures the domU attempting to
perform p2m accesses while the p2m changes are taking place will always
access the BAR contents. This is not the current behavior for domUs, so add
a note that it would preferably done that way.
Signed-off-by: Roger Pau Monné <roger....@citrix.com>
---
xen/drivers/vpci/header.c | 42 +++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 1035dcca242d..1a501a0ba47e 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -163,8 +163,6 @@ static void modify_decoding(const struct pci_dev *pdev,
uint16_t cmd,
if ( pci_check_bar(pdev, _mfn(PFN_DOWN(bar->addr)),
_mfn(PFN_DOWN(bar->addr + bar->size - 1))) )
bar->enabled = map;
- header->rom_enabled = map;
- pci_conf_write32(pdev->sbdf, rom_pos, val);
return;
}
@@ -174,14 +172,6 @@ static void modify_decoding(const struct pci_dev *pdev,
uint16_t cmd,
_mfn(PFN_DOWN(bar->addr + bar->size - 1))) )
bar->enabled = map;
}
-
- if ( !rom_only )
- {
- pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
- header->bars_mapped = map;
- }
- else
- ASSERT_UNREACHABLE();
}
bool vpci_process_pending(struct vcpu *v)
@@ -547,16 +537,23 @@ static void cf_check cmd_write(
* decoding one. Bits that are not allowed for DomU are already
* handled above and by the rsvdp_mask.
*/
- if ( header->bars_mapped != !!(cmd & PCI_COMMAND_MEMORY) )
+ if ( header->bars_mapped != !!(cmd & PCI_COMMAND_MEMORY) &&
+ modify_bars(pdev, cmd, false) )
/*
* Ignore the error. No memory has been added or removed from the p2m
* (because the actual p2m changes are deferred in defer_map) and the
* memory decoding bit has not been changed, so leave everything as-is,
* hoping the guest will realize and try again.
*/
- modify_bars(pdev, cmd, false);
- else
- pci_conf_write16(pdev->sbdf, reg, cmd);
+ return;
+
+ /*
+ * FIXME: for domUs we don't want the guest toggling the memory decoding
+ * bit. It should be set in vpci_init_header() and guest attempts to
+ * modify it should only lead to guest p2m changes.
+ */
+ header->bars_mapped = cmd & PCI_COMMAND_MEMORY;
+ pci_conf_write16(pdev->sbdf, reg, cmd);
}
static uint32_t cf_check guest_cmd_read(
@@ -712,17 +709,11 @@ static void cf_check rom_write(
rom->guest_addr = rom->addr;
}
- if ( !header->bars_mapped || rom->enabled == new_enabled )
- {
- /* Just update the ROM BAR field. */
- header->rom_enabled = new_enabled;
- pci_conf_write32(pdev->sbdf, reg, val);
- }
/*
* Pass PCI_COMMAND_MEMORY or 0 to signal a map/unmap request, note that
* this fabricated command is never going to be written to the register.
*/
- else if ( modify_bars(pdev, new_enabled ? PCI_COMMAND_MEMORY : 0, true) )
+ if ( modify_bars(pdev, new_enabled ? PCI_COMMAND_MEMORY : 0, true) )
/*
* No memory has been added or removed from the p2m (because the actual
* p2m changes are deferred in defer_map) and the ROM enable bit has
@@ -733,6 +724,8 @@ static void cf_check rom_write(
*/
return;
+ header->rom_enabled = new_enabled;
+ pci_conf_write32(pdev->sbdf, reg, val);
if ( !new_enabled )
{
rom->addr = val & PCI_ROM_ADDRESS_MASK;
@@ -1057,6 +1050,13 @@ int vpci_init_header(struct pci_dev *pdev)
goto fail;
}
+ if ( cmd & PCI_COMMAND_MEMORY )
+ {
+ /* Restore command register value. */
+ header->bars_mapped = true;
+ pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
+ }
+
return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
fail:
--
2.49.0
