[Qemu-devel] PATCH/RFC: PCI memory mapping

[Qemu-devel] PATCH/RFC: PCI memory mapping

[Brian Wheeler]

[first off, if there's an easier way to do this, let me know!]

This patch adds an address mapping function to the PCI bus so the host
chipset can remap PCI generated addresses to the appropriate physical
addresses.  

It adds two parameters to pci_register_bus:

PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                         qemu_irq *pic, int devfn_min, int nirq,
			 pci_map_addr_fn map_addr, void *map_addr_opaque)

The map_addr is a function which has the prototype:

typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, int bus_num, 
					      target_phys_addr_t addr);


and the map_addr_opaque data is a pointer to the IOTLB buffer in the
chipset.

When a pci device is doing dma or needs to write to the system, it can
use the

target_phys_addr_t pci_phys_addr(PCIDevice *device,
				 target_phys_addr_t addr);

function to get the mapped target address.  

If map_addr is NULL the function just returns the address it was passed.
Otherwise the map_addr function is called and the address is translated.

Thoughts/comments/etc?

Brian

--- qemu/hw/pci.c	2009-03-25 15:00:23.000000000 -0400
+++ qemu-alpha-20090330/hw/pci.c	2009-04-02 10:19:31.000000000 -0400
@@ -29,12 +29,15 @@
 #include "sysemu.h"
 
 //#define DEBUG_PCI
+//#define DEBUG_PCI_MAP_ADDR
 
 struct PCIBus {
     int bus_num;
     int devfn_min;
     pci_set_irq_fn set_irq;
     pci_map_irq_fn map_irq;
+    pci_map_addr_fn map_addr;
+    void *map_addr_opaque;
     uint32_t config_reg; /* XXX: suppress */
     /* low level pic */
     SetIRQFunc *low_set_irq;
@@ -57,6 +60,7 @@
 static int pci_irq_index;
 static PCIBus *first_bus;
 
+
 static void pcibus_save(QEMUFile *f, void *opaque)
 {
     PCIBus *bus = (PCIBus *)opaque;
@@ -89,7 +93,8 @@
 }
 
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         qemu_irq *pic, int devfn_min, int nirq)
+                         qemu_irq *pic, int devfn_min, int nirq,
+			 pci_map_addr_fn map_addr, void *map_addr_opaque)
 {
     PCIBus *bus;
     static int nbus = 0;
@@ -100,6 +105,8 @@
     bus->irq_opaque = pic;
     bus->devfn_min = devfn_min;
     bus->nirq = nirq;
+    bus->map_addr = map_addr;
+    bus->map_addr_opaque = map_addr_opaque;
     first_bus = bus;
     register_savevm("PCIBUS", nbus++, 1, pcibus_save, pcibus_load, bus);
     return bus;
@@ -656,6 +663,24 @@
 }
 
 /***********************************************************/
+/* Generic pci memory mapping support */
+target_phys_addr_t pci_phys_addr(PCIDevice *device,
+				 target_phys_addr_t addr) {
+  target_phys_addr_t physaddr;
+  PCIBus *bus = device->bus;
+  if(bus->map_addr == NULL)
+    physaddr = addr + pci_mem_base;
+  else 
+    physaddr = bus->map_addr(bus->map_addr_opaque, bus->bus_num, addr);
+
+#ifdef DEBUG_PCI_MAP_ADDR
+  printf("PCI Physical Memory mapping:  %08lx -> %08lx\n", addr, physaddr);
+#endif
+  return physaddr;
+}
+
+
+/***********************************************************/
 /* monitor info on PCI */
 
 typedef struct {
--- qemu/hw/pci.h	2009-03-25 15:00:23.000000000 -0400
+++ qemu-alpha-20090330/hw/pci.h	2009-04-02 10:19:07.000000000 -0400
@@ -169,8 +176,10 @@
 
 typedef void (*pci_set_irq_fn)(qemu_irq *pic, int irq_num, int level);
 typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
+typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, int bus_num, 
+					      target_phys_addr_t addr);
 PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-                         qemu_irq *pic, int devfn_min, int nirq);
+                         qemu_irq *pic, int devfn_min, int nirq, pci_map_addr_fn map_addr, void *map_addr_opaque);
 
 PCIDevice *pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn,
                   const char *default_model);
@@ -205,6 +214,9 @@
 {
     cpu_to_le16wu((uint16_t *)&pci_config[PCI_CLASS_DEVICE], val);
 }
+target_phys_addr_t pci_phys_addr(PCIDevice *device,
+				 target_phys_addr_t addr);
+
 
 /* lsi53c895a.c */
 #define LSI_MAX_DEVS 7

[Qemu-devel] Re: PATCH/RFC: PCI memory mapping

[Tristan Gingold]

On Apr 2, 2009, at 4:57 PM, Brian Wheeler wrote:

> [first off, if there's an easier way to do this, let me know!]
>
> This patch adds an address mapping function to the PCI bus so the host
> chipset can remap PCI generated addresses to the appropriate physical
> addresses.

I have been thinking about this for a while as many 64bits machines  
have an IOTLB or IOMMU.
And with VTd and AMD IOMMU it is becoming common.

> It adds two parameters to pci_register_bus:
>
> PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn  
> map_irq,
>                         qemu_irq *pic, int devfn_min, int nirq,
> 			 pci_map_addr_fn map_addr, void *map_addr_opaque)

map_addr_opaque should be simply opaque (in case of new callbacks are  
added).

> The map_addr is a function which has the prototype:
>
> typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, int  
> bus_num,
> 					      target_phys_addr_t addr);

I think this is not complete enough.

I'd replace int bus_num with PCIDevice *dev.  You can get the pci bus  
from the device.
The IOTLB mapping may depend on the device so we really need it.

Second you need to pass and return a length as the mapping is valid  
only for a certain length.

In the future the returned length could be 0 to simulate a pci abort...

> and the map_addr_opaque data is a pointer to the IOTLB buffer in the
> chipset.
>
> When a pci device is doing dma or needs to write to the system, it can
> use the
>
> target_phys_addr_t pci_phys_addr(PCIDevice *device,
> 				 target_phys_addr_t addr);
>
> function to get the mapped target address.
>
> If map_addr is NULL the function just returns the address it was  
> passed.
> Otherwise the map_addr function is called and the address is  
> translated.

I think we should do this in new functions such as pci_memory_read/ 
pci_memory_write which will
replace cpu_physical_memory_read/cpu_physical_memory_write.


Thank you for working on this,
Tristan.

Re: [Qemu-devel] Re: PATCH/RFC: PCI memory mapping

[Brian Wheeler]

On Thu, 2009-04-02 at 17:18 +0200, Tristan Gingold wrote:
> On Apr 2, 2009, at 4:57 PM, Brian Wheeler wrote:
> 
> > [first off, if there's an easier way to do this, let me know!]
> >
> > This patch adds an address mapping function to the PCI bus so the host
> > chipset can remap PCI generated addresses to the appropriate physical
> > addresses.
> 
> I have been thinking about this for a while as many 64bits machines  
> have an IOTLB or IOMMU.
> And with VTd and AMD IOMMU it is becoming common.
> 
> > It adds two parameters to pci_register_bus:
> >
> > PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn  
> > map_irq,
> >                         qemu_irq *pic, int devfn_min, int nirq,
> > 			 pci_map_addr_fn map_addr, void *map_addr_opaque)
> 
> map_addr_opaque should be simply opaque (in case of new callbacks are  
> added).
> 

Are you sure?  pic is the opaque data passed to pci_map_irq_fn, so we're
already setting more than one callback.  I've done it differently in the
new patch.


> > The map_addr is a function which has the prototype:
> >
> > typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, int  
> > bus_num,
> > 					      target_phys_addr_t addr);
> 
> I think this is not complete enough.
> 
> I'd replace int bus_num with PCIDevice *dev.  You can get the pci bus  
> from the device.
> The IOTLB mapping may depend on the device so we really need it.
> 

You can't actually get the bus number outside of pci.c.



> Second you need to pass and return a length as the mapping is valid  
> only for a certain length.
> 

I agree, as well as read/write access.



> In the future the returned length could be 0 to simulate a pci abort...
> 
> > and the map_addr_opaque data is a pointer to the IOTLB buffer in the
> > chipset.
> >
> > When a pci device is doing dma or needs to write to the system, it can
> > use the
> >
> > target_phys_addr_t pci_phys_addr(PCIDevice *device,
> > 				 target_phys_addr_t addr);
> >
> > function to get the mapped target address.
> >
> > If map_addr is NULL the function just returns the address it was  
> > passed.
> > Otherwise the map_addr function is called and the address is  
> > translated.
> 
> I think we should do this in new functions such as pci_memory_read/ 
> pci_memory_write which will
> replace cpu_physical_memory_read/cpu_physical_memory_write.
> 

Ok, so how about this patch. 

* new iommu API created:

#define PCI_IOMMU_MAP_INVALID 1
#define PCI_IOMMU_MAP_VALID 0
typedef int (*pci_map_addr_fn)(void *opaque, PCIDevice *device, 
			       target_phys_addr_t addr, ram_addr_t len,
			       int is_write, target_phys_addr_t *physaddr);
void pci_register_iommu(PCIBus *bus, pci_map_addr_fn map_addr, void *opaque);

int pci_iommu_memory_rw(PCIDevice *device, target_phys_addr_t addr,
			   uint8_t *buf, int len, int is_write);

static inline int pci_iommu_memory_read(PCIDevice *device, 
					   target_phys_addr_t addr,
					   uint8_t *buf, int len); 

static inline int pci_iommu_memory_write(PCIDevice *device, 
					    target_phys_addr_t addr,
					    const uint8_t *buf, int len);

int pci_register_iommu_memory_offset(PCIDevice *device,
					target_phys_addr_t start_addr,
					ram_addr_t size,
					ram_addr_t phys_offset,
					ram_addr_t region_offset);

static inline int pci_register_iommu_memory(PCIDevice *device, 
				 target_phys_addr_t start_addr,
				 ram_addr_t size,
				 ram_addr_t phys_offset);

int pci_get_bus_num(PCIDevice *device);


* minimal impact to the codebase, while providing new functionality:
** pci_register_bus argument list unchanged
** drivers converted to IOMMU will work ok even if the system doesn't
register an IOMMU device

What do you guys think?

Brian


Signed-off-by: Brian Wheeler <bdwheele@indiana.edu>

--- qemu/hw/pci.c	2009-03-25 15:00:23.000000000 -0400
+++ qemu-alpha-20090330/hw/pci.c	2009-04-02 13:11:34.000000000 -0400
@@ -29,12 +29,15 @@
 #include "sysemu.h"
 
 //#define DEBUG_PCI
+//#define DEBUG_PCI_IOMMU
 
 struct PCIBus {
     int bus_num;
     int devfn_min;
     pci_set_irq_fn set_irq;
     pci_map_irq_fn map_irq;
+    pci_map_addr_fn map_addr;
+    void *map_addr_opaque;
     uint32_t config_reg; /* XXX: suppress */
     /* low level pic */
     SetIRQFunc *low_set_irq;
@@ -57,6 +60,7 @@
 static int pci_irq_index;
 static PCIBus *first_bus;
 
+
 static void pcibus_save(QEMUFile *f, void *opaque)
 {
     PCIBus *bus = (PCIBus *)opaque;
@@ -100,6 +104,8 @@
     bus->irq_opaque = pic;
     bus->devfn_min = devfn_min;
     bus->nirq = nirq;
+    bus->map_addr = NULL;
+    bus->map_addr_opaque =NULL;
     first_bus = bus;
     register_savevm("PCIBUS", nbus++, 1, pcibus_save, pcibus_load, bus);
     return bus;
@@ -656,6 +662,62 @@
 }
 
 /***********************************************************/
+/* Generic pci memory mapping support */
+
+void pci_register_iommu(PCIBus *bus, pci_map_addr_fn map_addr, void *opaque) {
+    bus->map_addr = map_addr;
+    bus->map_addr_opaque = opaque;
+}
+
+int pci_register_iommu_memory_offset(PCIDevice *device,
+					target_phys_addr_t start_addr,
+					ram_addr_t size,
+					ram_addr_t phys_offset,
+					ram_addr_t region_offset)
+{
+  target_phys_addr_t physaddr = start_addr;
+  if(device->bus->map_addr != NULL) {
+    int result = device->bus->map_addr(device->bus->map_addr_opaque,
+				       device, start_addr, size, 1, &physaddr);
+    if(result==PCI_IOMMU_MAP_INVALID) {
+#ifdef PCI_DEBUG_IOMMU
+      printf("(register) PCI IOMMU: Mapping %x failed.\n", start_addr);
+#endif
+      return result;
+    }
+  }
+  cpu_register_physical_memory_offset(start_addr, size, phys_offset, 
+				      region_offset);
+  return PCI_IOMMU_MAP_VALID;
+}
+
+int pci_iommu_memory_rw(PCIDevice *device, target_phys_addr_t addr,
+			   uint8_t *buf, int len, int is_write)
+{
+  target_phys_addr_t physaddr = addr + pci_mem_base;
+  if(device->bus->map_addr != NULL) {
+    int result = device->bus->map_addr(device->bus->map_addr_opaque,
+				       device, addr, len, is_write, &physaddr);
+    if(result==PCI_IOMMU_MAP_INVALID) {
+#ifdef PCI_DEBUG_IOMMU
+      printf("(rw) PCI IOMMU: Mapping %x failed.\n", addr);
+#endif
+      return result;
+    }
+  }
+#ifdef PCI_DEBUG_IOMMU
+  printf("(rw) PCI IOMMU:  %x -> %x \n", addr, physaddr);
+#endif
+  cpu_physical_memory_rw(physaddr, buf, len, is_write);
+  return PCI_IOMMU_MAP_VALID;
+}
+
+int pci_get_bus_num(PCIDevice *device) {
+  return device->bus->bus_num;
+}
+
+
+/***********************************************************/
 /* monitor info on PCI */
 
 typedef struct {
--- qemu/hw/pci.h	2009-03-25 15:00:23.000000000 -0400
+++ qemu-alpha-20090330/hw/pci.h	2009-04-02 12:57:50.000000000 -0400
@@ -206,6 +214,48 @@
     cpu_to_le16wu((uint16_t *)&pci_config[PCI_CLASS_DEVICE], val);
 }
 
+/* PCI Memory Mapping */
+#define PCI_IOMMU_MAP_INVALID 1
+#define PCI_IOMMU_MAP_VALID 0
+typedef int (*pci_map_addr_fn)(void *opaque, PCIDevice *device, 
+			       target_phys_addr_t addr, ram_addr_t len,
+			       int is_write, target_phys_addr_t *physaddr);
+void pci_register_iommu(PCIBus *bus, pci_map_addr_fn map_addr, void *opaque);
+
+int pci_iommu_memory_rw(PCIDevice *device, target_phys_addr_t addr,
+			   uint8_t *buf, int len, int is_write);
+
+static inline int pci_iommu_memory_read(PCIDevice *device, 
+					   target_phys_addr_t addr,
+					   uint8_t *buf, int len) 
+{
+  return pci_iommu_memory_rw(device, addr, buf, len, 0);
+}
+
+static inline int pci_iommu_memory_write(PCIDevice *device, 
+					    target_phys_addr_t addr,
+					    const uint8_t *buf, int len) 
+{
+  return pci_iommu_memory_rw(device, addr, (uint8_t *)buf, len, 1);
+}
+
+int pci_register_iommu_memory_offset(PCIDevice *device,
+					target_phys_addr_t start_addr,
+					ram_addr_t size,
+					ram_addr_t phys_offset,
+					ram_addr_t region_offset);
+
+static inline int pci_register_iommu_memory(PCIDevice *device, 
+				 target_phys_addr_t start_addr,
+				 ram_addr_t size,
+				 ram_addr_t phys_offset) {
+  return pci_register_iommu_memory_offset(device, start_addr, size, 
+					     phys_offset, 0);
+}
+
+int pci_get_bus_num(PCIDevice *device);
+
+
 /* lsi53c895a.c */
 #define LSI_MAX_DEVS 7
 void lsi_scsi_attach(void *opaque, BlockDriverState *bd, int id);

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Blue Swirl]

On 4/2/09, Brian Wheeler <bdwheele@indiana.edu> wrote:
> [first off, if there's an easier way to do this, let me know!]
>
>  This patch adds an address mapping function to the PCI bus so the host
>  chipset can remap PCI generated addresses to the appropriate physical
>  addresses.

This discussion has come up many, many times, but finally the design
was settled to the current dma API. It supports resolving guest
addresses to host pointers, so  that part should be extended instead
of adding new functionality. We want (one day) zero copy DMA even with
IOMMU.

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Brian Wheeler]

On Thu, 2009-04-02 at 20:32 +0300, Blue Swirl wrote:
> On 4/2/09, Brian Wheeler <bdwheele@indiana.edu> wrote:
> > [first off, if there's an easier way to do this, let me know!]
> >
> >  This patch adds an address mapping function to the PCI bus so the host
> >  chipset can remap PCI generated addresses to the appropriate physical
> >  addresses.
> 
> This discussion has come up many, many times, but finally the design
> was settled to the current dma API. It supports resolving guest
> addresses to host pointers, so  that part should be extended instead
> of adding new functionality. We want (one day) zero copy DMA even with
> IOMMU.
> 
> 

Is there documentation somewhere?

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Paul Brook]

On Thursday 02 April 2009, Brian Wheeler wrote:
> [first off, if there's an easier way to do this, let me know!]
>
> This patch adds an address mapping function to the PCI bus so the host
> chipset can remap PCI generated addresses to the appropriate physical
> addresses.

This should be integrated with the DMA APIs.  Individual devices must not need 
to be aware of bus bridges. All remapping should be handled by the DMA 
routines.  In principle there's no reason for this to be specific to PCI. 
SPARC machines already have an IOMMU, so you should make sure your solution 
also covers those systems.

I recommend looking back in the list archives, there have been several 
discussions about the requirements for bus address mapping.

Paul

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Brian Wheeler]

On Thu, 2009-04-02 at 22:40 +0000, Paul Brook wrote:
> On Thursday 02 April 2009, Brian Wheeler wrote:
> > [first off, if there's an easier way to do this, let me know!]
> >
> > This patch adds an address mapping function to the PCI bus so the host
> > chipset can remap PCI generated addresses to the appropriate physical
> > addresses.
> 
> This should be integrated with the DMA APIs.  Individual devices must not need 
> to be aware of bus bridges. All remapping should be handled by the DMA 
> routines.  In principle there's no reason for this to be specific to PCI. 
> SPARC machines already have an IOMMU, so you should make sure your solution 
> also covers those systems.
> 
> I recommend looking back in the list archives, there have been several 
> discussions about the requirements for bus address mapping.
> 
> Paul


I've been looking around the archives and I'm not finding what I'm
looking for...though it may be that words like dma, address, etc are
terribly generic in this context.  I've found a few promising patches in
the archives, but they don't seem to be in the code.

Here's the problem I'm trying to solve:  The alpha has an IOMMU which
translates pci addresses to addresses which are used by the system.  The
first mapping is for local connect addresses which work out to:
   (0x80000000000ULL | (bus * 0x200000000ULL) | address

Another mapping uses scatter-gather.  A third one uses a direct connect
mapping which is a base/range => new address mapping.

My question is:  where/how do I hook my function to translate the
address from cpu_physical_memory_{register,read,write}() as called by
the different devices into the addresses that I need?

So, for example, a cirrus vga device on bus 1 when calling:

cpu_register_physical_memory(addr, s->vram_size,
s->cirrus_linear_io_addr);

The address actually gets mapped to the alpha's address
0x80200000000 + addr?

How do other architectures do this?  Looking around the source, I'm not
finding anything that stands out....


Thanks, and sorry for the newbie questions!
Brian

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Paul Brook]

> So, for example, a cirrus vga device on bus 1 when calling:
>
> cpu_register_physical_memory(addr, s->vram_size,
> s->cirrus_linear_io_addr);

Ah, that's a different (but somewhat related) problem. This is for accesses 
from the CPU to the device. Most current IOMMU effect device->CPU 
accesses[1].

Paul

[1] For practical purposes assume the the CPU and system ram are on the same 
bus.

Re: [Qemu-devel] PATCH/RFC: PCI memory mapping

[Brian Wheeler]

On Fri, 2009-04-03 at 15:28 +0000, Paul Brook wrote:
> > So, for example, a cirrus vga device on bus 1 when calling:
> >
> > cpu_register_physical_memory(addr, s->vram_size,
> > s->cirrus_linear_io_addr);
> 
> Ah, that's a different (but somewhat related) problem. This is for accesses 
> from the CPU to the device. Most current IOMMU effect device->CPU 
> accesses[1].
> 

Ok, so is there an official work around for this one?  Right now we've
hacked around it by just adding pci_mem_base to it during that call but
that limits us to one bus and it requires patching all of the pci
devices we use.

Would a patch implementing this API be acceptable?

--------------
typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, PCIDevice *device, 
                            target_phys_addr_t addr, ram_addr_t len);
void pci_register_address_mapper(PCIBus *bus, pci_map_addr_fn map_addr, void *opaque);
static inline int pci_register_physical_memory(PCIDevice *device, 
                                 target_phys_addr_t start_addr,
                                 ram_addr_t size,
                                 ram_addr_t phys_offset);
----------------

The pci_register_physical_memory would call the callback (if any) to
generate the appropriate physical address.

So, the cirrus example above would be changed to:

pci_register_physical_memory(addr, s->vram_size,
                             s->cirrus_linear_io_addr);

which would (on the PC and others) just map addr to itself.  On any
architecture which registered an address mapper, the address actually
mapped would be generated.


Thanks!
Brian






> Paul
> 
> [1] For practical purposes assume the the CPU and system ram are on the same 
> bus.