[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] hw/pci-host/versatile: Add WINDOW_COUNT definition to re

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 3/4] hw/pci-host/versatile: Add WINDOW_COUNT definition to replace magic '3'
Date: Mon, 12 Oct 2020 15:01:51 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 10/11/20 10:46 PM, Peter Maydell wrote:
On Sun, 11 Oct 2020 at 20:49, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

Use self-explicit WINDOW_COUNT definition instead of a magic value.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
  hw/pci-host/versatile.c | 28 ++++++++++++++--------------
  1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index b4951023f4e..4d9565de4b1 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -72,6 +72,8 @@ enum {

+#define WINDOW_COUNT 3
  struct PCIVPBState {
      PCIHostState parent_obj;

@@ -86,18 +88,18 @@ struct PCIVPBState {
       * The offsets into pci_mem_space are controlled by the imap registers.
      MemoryRegion pci_io_window;
-    MemoryRegion pci_mem_window[3];
+    MemoryRegion pci_mem_window[WINDOW_COUNT];
      PCIBus pci_bus;
      PCIDevice pci_dev;

      /* Constant for life of device: */
      int realview;
-    uint32_t mem_win_size[3];
+    uint32_t mem_win_size[WINDOW_COUNT];
      uint8_t irq_mapping_prop;

      /* Variable state: */
-    uint32_t imap[3];
-    uint32_t smap[3];
+    uint32_t imap[WINDOW_COUNT];
+    uint32_t smap[WINDOW_COUNT];

Strictly speaking, this is conflating two separate
things which both happen to be equal to three.

The versatile/realview PCI controller has:
  * three memory windows in the system address space
    - those are represented by the pci_mem_window[] array
    - mem_win_size[] holds the size of each window
      (which is fixed but varies between the different
      implementations of this controller on different boards)
    - the device IMAPn registers allow the guest to
      configure the mapping from "a CPU access to an
      address in window n" to "a PCI address on the PCI bus,
      and our imap[] array holds those register values
  * three PCI BARs which represent memory windows on the
      PCI bus which bus-master PCI devices can use to
      write back into the system address space
    - the device SMAPn registers allow the guest to configure
      the mapping from "a bus-master access to an address
      on the PCI bus wherever the guest mapped BAR n"
      to "a system address", and the smap[] array holds
      those register values
There's no inherent reason why the number of PCI BARs
needs to be the same as the number of system address
space memory windows, so they shouldn't really share
the same constant.

Thanks for the detailed explanation, I'll update.

(We don't actually implement the behaviour of the SMAP
registers and the BARs, because Linux always configures
the PCI controller to a 1:1 mapping of PCI space to
system address space. So we get away with just having
our implementation be "always do direct accesses".)

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]