pci: Fix the update of interrupt disable bit in PCI_COMMAND register

pci: Fix the update of interrupt disable bit in PCI_COMMAND register

[Guoyi Tu]

The PCI_COMMAND register is located at offset 4 within
the PCI configuration space and occupies 2 bytes. The
interrupt disable bit is at the 10th bit, which corresponds
to the byte at offset 5 in the PCI configuration space.

In our testing environment, the guest driver may directly
updates the byte at offset 5 in the PCI configuration space.
The backtrace looks like as following:
#0  pci_default_write_config (d=0x5580bbfc6230, addr=5, val_in=5, l=1)
     at hw/pci/pci.c:1442
#1  0x00005580b8f3156a in virtio_write_config (pci_dev=0x5580bbfc6230, 
address=5, val=5, len=1)
     at hw/virtio/virtio-pci.c:605
#2  0x00005580b8ed2f3b in pci_host_config_write_common 
(pci_dev=0x5580bbfc6230, addr=5, limit=256,
     val=5, len=1) at hw/pci/pci_host.c:81

In this situation, the range_covers_byte function called
by the pci_default_write_config function will return false,
resulting in the inability to handle the interrupt disable
update event.

To fix this issue, we can use the ranges_overlap function
instead of range_covers_byte to determine whether the interrupt
bit has been updated.

Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
Signed-off-by: yuanminghao <yuanmh12@chinatelecom.cn>
---
  hw/pci/pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b8d22e2e74..881d774fb6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1613,7 +1613,7 @@ void pci_default_write_config(PCIDevice *d, 
uint32_t addr, uint32_t val_in, int
          range_covers_byte(addr, l, PCI_COMMAND))
          pci_update_mappings(d);

-    if (range_covers_byte(addr, l, PCI_COMMAND)) {
+    if (ranges_overlap(addr, l, PCI_COMMAND, 2)) {
          pci_update_irq_disabled(d, was_irq_disabled);
          memory_region_set_enabled(&d->bus_master_enable_region,
                                    (pci_get_word(d->config + PCI_COMMAND)
-- 
2.27.0

--
Guoyi

Re: pci: Fix the update of interrupt disable bit in PCI_COMMAND register

[Michael S. Tsirkin]

On Fri, Aug 11, 2023 at 10:46:51PM +0800, Guoyi Tu wrote:
> The PCI_COMMAND register is located at offset 4 within
> the PCI configuration space and occupies 2 bytes. The
> interrupt disable bit is at the 10th bit, which corresponds
> to the byte at offset 5 in the PCI configuration space.
> 
> In our testing environment, the guest driver may directly
> updates the byte at offset 5 in the PCI configuration space.
> The backtrace looks like as following:
> #0  pci_default_write_config (d=0x5580bbfc6230, addr=5, val_in=5, l=1)
>     at hw/pci/pci.c:1442
> #1  0x00005580b8f3156a in virtio_write_config (pci_dev=0x5580bbfc6230,
> address=5, val=5, len=1)
>     at hw/virtio/virtio-pci.c:605
> #2  0x00005580b8ed2f3b in pci_host_config_write_common
> (pci_dev=0x5580bbfc6230, addr=5, limit=256,
>     val=5, len=1) at hw/pci/pci_host.c:81
> 
> In this situation, the range_covers_byte function called
> by the pci_default_write_config function will return false,
> resulting in the inability to handle the interrupt disable
> update event.
> 
> To fix this issue, we can use the ranges_overlap function
> instead of range_covers_byte to determine whether the interrupt
> bit has been updated.
> 
> Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
> Signed-off-by: yuanminghao <yuanmh12@chinatelecom.cn>

Oh wow good catch!

Fixes: b6981cb57be5 ("pci: interrupt disable bit support")

clearly stable material too.


> ---
>  hw/pci/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b8d22e2e74..881d774fb6 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1613,7 +1613,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t
> addr, uint32_t val_in, int
>          range_covers_byte(addr, l, PCI_COMMAND))
>          pci_update_mappings(d);
> 
> -    if (range_covers_byte(addr, l, PCI_COMMAND)) {
> +    if (ranges_overlap(addr, l, PCI_COMMAND, 2)) {
>          pci_update_irq_disabled(d, was_irq_disabled);
>          memory_region_set_enabled(&d->bus_master_enable_region,
>                                    (pci_get_word(d->config + PCI_COMMAND)
> -- 
> 2.27.0
> 
> --
> Guoyi