[RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Eduard - Gabriel Munteanu]

Hi everybody,

This patchset is intended to provide a start for implementing the
emulation of the AMD IOMMU. For those who aren't aware yet, I intend
to participate as a student in GSoC 2010.

The patches are meant to be applied on top of qemu-kvm.

In short, this demonstrates a mechanism of inserting ACPI tables without
modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
equivalent, but I think this approach is better, at least at the moment.

To test, simply boot a Linux kernel inside KVM and look in dmesg for the
IVRS table.

I wouldn't merge this patchset yet, at least stuff after the first patch,
until it accumulates more work. I also didn't test loading ACPI tables from
the command line after these modifications.

I'd appreciate comments on these patches.


	Cheers,
	Eduard

Eduard - Gabriel Munteanu (7):
  acpi: qemu_realloc() might return a different pointer
  acpi: split and rename acpi_table_add()
  acpi: move table header definition into pc.h
  sparc: rename hw/iommu.c
  x86-64: AMD IOMMU stub
  acpi: cleanup acpi_checksum()
  acpi: fix bug in acpi_checksum() caused by garbage in checksum field

 Makefile.target               |    3 +-
 hw/acpi.c                     |   83 +++++++++++++++++++-------------
 hw/amd_iommu.c                |  103 +++++++++++++++++++++++++++++++++++++++++
 hw/pc.c                       |    2 +
 hw/pc.h                       |   20 ++++++++-
 hw/{iommu.c => sparc_iommu.c} |    0
 hw/sun4m.h                    |    2 +-
 vl.c                          |    2 +-
 8 files changed, 177 insertions(+), 38 deletions(-)
 create mode 100644 hw/amd_iommu.c
 rename hw/{iommu.c => sparc_iommu.c} (100%)

[Qemu-devel] [RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Eduard - Gabriel Munteanu]

Hi everybody,

This patchset is intended to provide a start for implementing the
emulation of the AMD IOMMU. For those who aren't aware yet, I intend
to participate as a student in GSoC 2010.

The patches are meant to be applied on top of qemu-kvm.

In short, this demonstrates a mechanism of inserting ACPI tables without
modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
equivalent, but I think this approach is better, at least at the moment.

To test, simply boot a Linux kernel inside KVM and look in dmesg for the
IVRS table.

I wouldn't merge this patchset yet, at least stuff after the first patch,
until it accumulates more work. I also didn't test loading ACPI tables from
the command line after these modifications.

I'd appreciate comments on these patches.


	Cheers,
	Eduard

Eduard - Gabriel Munteanu (7):
  acpi: qemu_realloc() might return a different pointer
  acpi: split and rename acpi_table_add()
  acpi: move table header definition into pc.h
  sparc: rename hw/iommu.c
  x86-64: AMD IOMMU stub
  acpi: cleanup acpi_checksum()
  acpi: fix bug in acpi_checksum() caused by garbage in checksum field

 Makefile.target               |    3 +-
 hw/acpi.c                     |   83 +++++++++++++++++++-------------
 hw/amd_iommu.c                |  103 +++++++++++++++++++++++++++++++++++++++++
 hw/pc.c                       |    2 +
 hw/pc.h                       |   20 ++++++++-
 hw/{iommu.c => sparc_iommu.c} |    0
 hw/sun4m.h                    |    2 +-
 vl.c                          |    2 +-
 8 files changed, 177 insertions(+), 38 deletions(-)
 create mode 100644 hw/amd_iommu.c
 rename hw/{iommu.c => sparc_iommu.c} (100%)

[RFC PATCH 1/7] acpi: qemu_realloc() might return a different pointer

[Eduard - Gabriel Munteanu]

We mustn't assume qemu_realloc() returns the same pointer in
acpi_table_add(). Therefore, 'p' might be invalid if it's relative to
the old value of acpi_tables.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index d293127..7c4e8d3 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -857,7 +857,7 @@ int acpi_table_add(const char *t)
     char buf[1024], *p, *f;
     struct acpi_table_header acpi_hdr;
     unsigned long val;
-    size_t off;
+    size_t newlen, off;
 
     memset(&acpi_hdr, 0, sizeof(acpi_hdr));
   
@@ -938,9 +938,10 @@ int acpi_table_add(const char *t)
         acpi_tables_len = sizeof(uint16_t);
         acpi_tables = qemu_mallocz(acpi_tables_len);
     }
+    newlen = acpi_tables_len + sizeof(uint16_t) + acpi_hdr.length;
+    acpi_tables = qemu_realloc(acpi_tables, newlen);
     p = acpi_tables + acpi_tables_len;
-    acpi_tables_len += sizeof(uint16_t) + acpi_hdr.length;
-    acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
+    acpi_tables_len = newlen;
 
     acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
     *(uint16_t*)p = acpi_hdr.length;
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 1/7] acpi: qemu_realloc() might return a different pointer

[Eduard - Gabriel Munteanu]

We mustn't assume qemu_realloc() returns the same pointer in
acpi_table_add(). Therefore, 'p' might be invalid if it's relative to
the old value of acpi_tables.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index d293127..7c4e8d3 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -857,7 +857,7 @@ int acpi_table_add(const char *t)
     char buf[1024], *p, *f;
     struct acpi_table_header acpi_hdr;
     unsigned long val;
-    size_t off;
+    size_t newlen, off;
 
     memset(&acpi_hdr, 0, sizeof(acpi_hdr));
   
@@ -938,9 +938,10 @@ int acpi_table_add(const char *t)
         acpi_tables_len = sizeof(uint16_t);
         acpi_tables = qemu_mallocz(acpi_tables_len);
     }
+    newlen = acpi_tables_len + sizeof(uint16_t) + acpi_hdr.length;
+    acpi_tables = qemu_realloc(acpi_tables, newlen);
     p = acpi_tables + acpi_tables_len;
-    acpi_tables_len += sizeof(uint16_t) + acpi_hdr.length;
-    acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
+    acpi_tables_len = newlen;
 
     acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
     *(uint16_t*)p = acpi_hdr.length;
-- 
1.6.4.4

[RFC PATCH 2/7] acpi: split and rename acpi_table_add()

[Eduard - Gabriel Munteanu]

We'd like to let emulation code build and insert ACPI tables at bootup,
without depending on hacking the BIOS code. This will be used to provide
an IVRS table for emulating the AMD IOMMU, for instance.

This splits acpi_table_add(), retaining the old behavior of inserting
cmdline-supplied tables under the name of acpi_table_cmdline_add(). The
other two resulting functions can be used for the aforementioned
purpose.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |   64 ++++++++++++++++++++++++++++++++++++++++---------------------
 hw/pc.h   |    4 ++-
 vl.c      |    2 +-
 3 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 7c4e8d3..8eb53da 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -840,7 +840,7 @@ struct acpi_table_header
 } __attribute__((packed));
 
 char *acpi_tables;
-size_t acpi_tables_len;
+size_t acpi_tables_len, acpi_tables_prev_len;
 
 static int acpi_checksum(const uint8_t *data, int len)
 {
@@ -851,13 +851,44 @@ static int acpi_checksum(const uint8_t *data, int len)
     return (-sum) & 0xff;
 }
 
-int acpi_table_add(const char *t)
+void *acpi_alloc_table(size_t size)
+{
+    void *ptr;
+
+    if (!acpi_tables) {
+        acpi_tables_len = sizeof(uint16_t);
+        acpi_tables = qemu_mallocz(acpi_tables_len);
+    }
+    acpi_tables_prev_len = acpi_tables_len;
+    acpi_tables_len += sizeof(uint16_t) + size;
+    acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
+    ptr = acpi_tables + acpi_tables_prev_len;
+
+    *(uint16_t *) ptr = size;
+
+    return ptr + sizeof(uint16_t);
+}
+
+void acpi_commit_table(void *buf)
+{
+    struct acpi_table_header *acpi_hdr = buf;
+    size_t size = acpi_tables_len - acpi_tables_prev_len - sizeof(uint16_t);
+
+    acpi_hdr->length = cpu_to_le32(size);
+    acpi_hdr->checksum = acpi_checksum(buf, size);
+
+    /* increase number of tables */
+    (*(uint16_t *) acpi_tables) =
+	    cpu_to_le32(le32_to_cpu(*(uint16_t *) acpi_tables) + 1);
+}
+
+int acpi_table_cmdline_add(const char *t)
 {
     static const char *dfl_id = "QEMUQEMU";
     char buf[1024], *p, *f;
     struct acpi_table_header acpi_hdr;
     unsigned long val;
-    size_t newlen, off;
+    size_t size, off;
 
     memset(&acpi_hdr, 0, sizeof(acpi_hdr));
   
@@ -915,7 +946,7 @@ int acpi_table_add(const char *t)
          buf[0] = '\0';
     }
 
-    acpi_hdr.length = sizeof(acpi_hdr);
+    size = sizeof(acpi_hdr);
 
     f = buf;
     while (buf[0]) {
@@ -927,27 +958,17 @@ int acpi_table_add(const char *t)
             fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno));
             goto out;
         }
-        acpi_hdr.length += s.st_size;
+        size += s.st_size;
         if (!n)
             break;
         *n = ':';
         f = n + 1;
     }
 
-    if (!acpi_tables) {
-        acpi_tables_len = sizeof(uint16_t);
-        acpi_tables = qemu_mallocz(acpi_tables_len);
-    }
-    newlen = acpi_tables_len + sizeof(uint16_t) + acpi_hdr.length;
-    acpi_tables = qemu_realloc(acpi_tables, newlen);
-    p = acpi_tables + acpi_tables_len;
-    acpi_tables_len = newlen;
+    p = acpi_alloc_table(size);
+    off = sizeof(struct acpi_table_header);
 
-    acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
-    *(uint16_t*)p = acpi_hdr.length;
-    p += sizeof(uint16_t);
-    memcpy(p, &acpi_hdr, sizeof(acpi_hdr));
-    off = sizeof(acpi_hdr);
+    memcpy(p, &acpi_hdr, off);
 
     f = buf;
     while (buf[0]) {
@@ -983,10 +1004,8 @@ int acpi_table_add(const char *t)
         f = n + 1;
     }
 
-    ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
-    /* increase number of tables */
-    (*(uint16_t*)acpi_tables) =
-	    cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1);
+    acpi_commit_table(p);
+
     return 0;
 out:
     if (acpi_tables) {
@@ -995,3 +1014,4 @@ out:
     }
     return -1;
 }
+
diff --git a/hw/pc.h b/hw/pc.h
index b599564..0cef140 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -108,7 +108,9 @@ extern char *acpi_tables;
 extern size_t acpi_tables_len;
 
 void acpi_bios_init(void);
-int acpi_table_add(const char *table_desc);
+void *acpi_alloc_table(size_t size);
+void acpi_commit_table(void *buf);
+int acpi_table_cmdline_add(const char *table_desc);
 
 /* acpi_piix.c */
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
diff --git a/vl.c b/vl.c
index d959fdb..0efba90 100644
--- a/vl.c
+++ b/vl.c
@@ -5492,7 +5492,7 @@ int main(int argc, char **argv, char **envp)
                 rtc_td_hack = 1;
                 break;
             case QEMU_OPTION_acpitable:
-                if(acpi_table_add(optarg) < 0) {
+                if(acpi_table_cmdline_add(optarg) < 0) {
                     fprintf(stderr, "Wrong acpi table provided\n");
                     exit(1);
                 }
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 2/7] acpi: split and rename acpi_table_add()

[Eduard - Gabriel Munteanu]

We'd like to let emulation code build and insert ACPI tables at bootup,
without depending on hacking the BIOS code. This will be used to provide
an IVRS table for emulating the AMD IOMMU, for instance.

This splits acpi_table_add(), retaining the old behavior of inserting
cmdline-supplied tables under the name of acpi_table_cmdline_add(). The
other two resulting functions can be used for the aforementioned
purpose.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |   64 ++++++++++++++++++++++++++++++++++++++++---------------------
 hw/pc.h   |    4 ++-
 vl.c      |    2 +-
 3 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 7c4e8d3..8eb53da 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -840,7 +840,7 @@ struct acpi_table_header
 } __attribute__((packed));
 
 char *acpi_tables;
-size_t acpi_tables_len;
+size_t acpi_tables_len, acpi_tables_prev_len;
 
 static int acpi_checksum(const uint8_t *data, int len)
 {
@@ -851,13 +851,44 @@ static int acpi_checksum(const uint8_t *data, int len)
     return (-sum) & 0xff;
 }
 
-int acpi_table_add(const char *t)
+void *acpi_alloc_table(size_t size)
+{
+    void *ptr;
+
+    if (!acpi_tables) {
+        acpi_tables_len = sizeof(uint16_t);
+        acpi_tables = qemu_mallocz(acpi_tables_len);
+    }
+    acpi_tables_prev_len = acpi_tables_len;
+    acpi_tables_len += sizeof(uint16_t) + size;
+    acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
+    ptr = acpi_tables + acpi_tables_prev_len;
+
+    *(uint16_t *) ptr = size;
+
+    return ptr + sizeof(uint16_t);
+}
+
+void acpi_commit_table(void *buf)
+{
+    struct acpi_table_header *acpi_hdr = buf;
+    size_t size = acpi_tables_len - acpi_tables_prev_len - sizeof(uint16_t);
+
+    acpi_hdr->length = cpu_to_le32(size);
+    acpi_hdr->checksum = acpi_checksum(buf, size);
+
+    /* increase number of tables */
+    (*(uint16_t *) acpi_tables) =
+	    cpu_to_le32(le32_to_cpu(*(uint16_t *) acpi_tables) + 1);
+}
+
+int acpi_table_cmdline_add(const char *t)
 {
     static const char *dfl_id = "QEMUQEMU";
     char buf[1024], *p, *f;
     struct acpi_table_header acpi_hdr;
     unsigned long val;
-    size_t newlen, off;
+    size_t size, off;
 
     memset(&acpi_hdr, 0, sizeof(acpi_hdr));
   
@@ -915,7 +946,7 @@ int acpi_table_add(const char *t)
          buf[0] = '\0';
     }
 
-    acpi_hdr.length = sizeof(acpi_hdr);
+    size = sizeof(acpi_hdr);
 
     f = buf;
     while (buf[0]) {
@@ -927,27 +958,17 @@ int acpi_table_add(const char *t)
             fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno));
             goto out;
         }
-        acpi_hdr.length += s.st_size;
+        size += s.st_size;
         if (!n)
             break;
         *n = ':';
         f = n + 1;
     }
 
-    if (!acpi_tables) {
-        acpi_tables_len = sizeof(uint16_t);
-        acpi_tables = qemu_mallocz(acpi_tables_len);
-    }
-    newlen = acpi_tables_len + sizeof(uint16_t) + acpi_hdr.length;
-    acpi_tables = qemu_realloc(acpi_tables, newlen);
-    p = acpi_tables + acpi_tables_len;
-    acpi_tables_len = newlen;
+    p = acpi_alloc_table(size);
+    off = sizeof(struct acpi_table_header);
 
-    acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
-    *(uint16_t*)p = acpi_hdr.length;
-    p += sizeof(uint16_t);
-    memcpy(p, &acpi_hdr, sizeof(acpi_hdr));
-    off = sizeof(acpi_hdr);
+    memcpy(p, &acpi_hdr, off);
 
     f = buf;
     while (buf[0]) {
@@ -983,10 +1004,8 @@ int acpi_table_add(const char *t)
         f = n + 1;
     }
 
-    ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
-    /* increase number of tables */
-    (*(uint16_t*)acpi_tables) =
-	    cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1);
+    acpi_commit_table(p);
+
     return 0;
 out:
     if (acpi_tables) {
@@ -995,3 +1014,4 @@ out:
     }
     return -1;
 }
+
diff --git a/hw/pc.h b/hw/pc.h
index b599564..0cef140 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -108,7 +108,9 @@ extern char *acpi_tables;
 extern size_t acpi_tables_len;
 
 void acpi_bios_init(void);
-int acpi_table_add(const char *table_desc);
+void *acpi_alloc_table(size_t size);
+void acpi_commit_table(void *buf);
+int acpi_table_cmdline_add(const char *table_desc);
 
 /* acpi_piix.c */
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
diff --git a/vl.c b/vl.c
index d959fdb..0efba90 100644
--- a/vl.c
+++ b/vl.c
@@ -5492,7 +5492,7 @@ int main(int argc, char **argv, char **envp)
                 rtc_td_hack = 1;
                 break;
             case QEMU_OPTION_acpitable:
-                if(acpi_table_add(optarg) < 0) {
+                if(acpi_table_cmdline_add(optarg) < 0) {
                     fprintf(stderr, "Wrong acpi table provided\n");
                     exit(1);
                 }
-- 
1.6.4.4

[RFC PATCH 3/7] acpi: move table header definition into pc.h

[Eduard - Gabriel Munteanu]

This moves the table header definition into pc.h to allow other code to
build ACPI tables.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |   13 -------------
 hw/pc.h   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 8eb53da..3794f70 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -826,19 +826,6 @@ static int piix4_device_hotplug(PCIDevice *dev, int state)
     return 0;
 }
 
-struct acpi_table_header
-{
-    char signature [4];    /* ACPI signature (4 ASCII characters) */
-    uint32_t length;          /* Length of table, in bytes, including header */
-    uint8_t revision;         /* ACPI Specification minor version # */
-    uint8_t checksum;         /* To make sum of entire table == 0 */
-    char oem_id [6];       /* OEM identification */
-    char oem_table_id [8]; /* OEM table identification */
-    uint32_t oem_revision;    /* OEM revision number */
-    char asl_compiler_id [4]; /* ASL compiler vendor ID */
-    uint32_t asl_compiler_revision; /* ASL compiler revision number */
-} __attribute__((packed));
-
 char *acpi_tables;
 size_t acpi_tables_len, acpi_tables_prev_len;
 
diff --git a/hw/pc.h b/hw/pc.h
index 0cef140..92954db 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -103,6 +103,19 @@ int ioport_get_a20(void);
 CPUState *pc_new_cpu(const char *cpu_model);
 
 /* acpi.c */
+struct acpi_table_header
+{
+    char signature [4];    /* ACPI signature (4 ASCII characters) */
+    uint32_t length;          /* Length of table, in bytes, including header */
+    uint8_t revision;         /* ACPI Specification minor version # */
+    uint8_t checksum;         /* To make sum of entire table == 0 */
+    char oem_id [6];       /* OEM identification */
+    char oem_table_id [8]; /* OEM table identification */
+    uint32_t oem_revision;    /* OEM revision number */
+    char asl_compiler_id [4]; /* ASL compiler vendor ID */
+    uint32_t asl_compiler_revision; /* ASL compiler revision number */
+} __attribute__((packed));
+
 extern int acpi_enabled;
 extern char *acpi_tables;
 extern size_t acpi_tables_len;
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 3/7] acpi: move table header definition into pc.h

[Eduard - Gabriel Munteanu]

This moves the table header definition into pc.h to allow other code to
build ACPI tables.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |   13 -------------
 hw/pc.h   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 8eb53da..3794f70 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -826,19 +826,6 @@ static int piix4_device_hotplug(PCIDevice *dev, int state)
     return 0;
 }
 
-struct acpi_table_header
-{
-    char signature [4];    /* ACPI signature (4 ASCII characters) */
-    uint32_t length;          /* Length of table, in bytes, including header */
-    uint8_t revision;         /* ACPI Specification minor version # */
-    uint8_t checksum;         /* To make sum of entire table == 0 */
-    char oem_id [6];       /* OEM identification */
-    char oem_table_id [8]; /* OEM table identification */
-    uint32_t oem_revision;    /* OEM revision number */
-    char asl_compiler_id [4]; /* ASL compiler vendor ID */
-    uint32_t asl_compiler_revision; /* ASL compiler revision number */
-} __attribute__((packed));
-
 char *acpi_tables;
 size_t acpi_tables_len, acpi_tables_prev_len;
 
diff --git a/hw/pc.h b/hw/pc.h
index 0cef140..92954db 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -103,6 +103,19 @@ int ioport_get_a20(void);
 CPUState *pc_new_cpu(const char *cpu_model);
 
 /* acpi.c */
+struct acpi_table_header
+{
+    char signature [4];    /* ACPI signature (4 ASCII characters) */
+    uint32_t length;          /* Length of table, in bytes, including header */
+    uint8_t revision;         /* ACPI Specification minor version # */
+    uint8_t checksum;         /* To make sum of entire table == 0 */
+    char oem_id [6];       /* OEM identification */
+    char oem_table_id [8]; /* OEM table identification */
+    uint32_t oem_revision;    /* OEM revision number */
+    char asl_compiler_id [4]; /* ASL compiler vendor ID */
+    uint32_t asl_compiler_revision; /* ASL compiler revision number */
+} __attribute__((packed));
+
 extern int acpi_enabled;
 extern char *acpi_tables;
 extern size_t acpi_tables_len;
-- 
1.6.4.4

[RFC PATCH 4/7] sparc: rename hw/iommu.c

[Eduard - Gabriel Munteanu]

hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
AMD IOMMU, which could lead to confusion unless we rename the former.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Makefile.target               |    2 +-
 hw/{iommu.c => sparc_iommu.c} |    0
 hw/sun4m.h                    |    2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename hw/{iommu.c => sparc_iommu.c} (100%)

diff --git a/Makefile.target b/Makefile.target
index 4d88543..cbe19a6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -305,7 +305,7 @@ obj-sparc-y += vga.o vga-pci.o
 obj-sparc-y += fdc.o mc146818rtc.o serial.o
 obj-sparc-y += cirrus_vga.o parallel.o
 else
-obj-sparc-y = sun4m.o lance.o tcx.o iommu.o slavio_intctl.o
+obj-sparc-y = sun4m.o lance.o tcx.o sparc_iommu.o slavio_intctl.o
 obj-sparc-y += slavio_timer.o slavio_misc.o fdc.o sparc32_dma.o
 obj-sparc-y += cs4231.o eccmemctl.o sbi.o sun4c_intctl.o
 endif
diff --git a/hw/iommu.c b/hw/sparc_iommu.c
similarity index 100%
rename from hw/iommu.c
rename to hw/sparc_iommu.c
diff --git a/hw/sun4m.h b/hw/sun4m.h
index ce97ee5..5007924 100644
--- a/hw/sun4m.h
+++ b/hw/sun4m.h
@@ -5,7 +5,7 @@
 
 /* Devices used by sparc32 system.  */
 
-/* iommu.c */
+/* sparc_iommu.c */
 void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
                                  uint8_t *buf, int len, int is_write);
 static inline void sparc_iommu_memory_read(void *opaque,
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Eduard - Gabriel Munteanu]

hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
AMD IOMMU, which could lead to confusion unless we rename the former.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Makefile.target               |    2 +-
 hw/{iommu.c => sparc_iommu.c} |    0
 hw/sun4m.h                    |    2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename hw/{iommu.c => sparc_iommu.c} (100%)

diff --git a/Makefile.target b/Makefile.target
index 4d88543..cbe19a6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -305,7 +305,7 @@ obj-sparc-y += vga.o vga-pci.o
 obj-sparc-y += fdc.o mc146818rtc.o serial.o
 obj-sparc-y += cirrus_vga.o parallel.o
 else
-obj-sparc-y = sun4m.o lance.o tcx.o iommu.o slavio_intctl.o
+obj-sparc-y = sun4m.o lance.o tcx.o sparc_iommu.o slavio_intctl.o
 obj-sparc-y += slavio_timer.o slavio_misc.o fdc.o sparc32_dma.o
 obj-sparc-y += cs4231.o eccmemctl.o sbi.o sun4c_intctl.o
 endif
diff --git a/hw/iommu.c b/hw/sparc_iommu.c
similarity index 100%
rename from hw/iommu.c
rename to hw/sparc_iommu.c
diff --git a/hw/sun4m.h b/hw/sun4m.h
index ce97ee5..5007924 100644
--- a/hw/sun4m.h
+++ b/hw/sun4m.h
@@ -5,7 +5,7 @@
 
 /* Devices used by sparc32 system.  */
 
-/* iommu.c */
+/* sparc_iommu.c */
 void sparc_iommu_memory_rw(void *opaque, target_phys_addr_t addr,
                                  uint8_t *buf, int len, int is_write);
 static inline void sparc_iommu_memory_read(void *opaque,
-- 
1.6.4.4

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Blue Swirl]

On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  AMD IOMMU, which could lead to confusion unless we rename the former.

I was also thinking of renaming the file some time ago. The correct
name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
several IO-UNITs instead. All Sun4m machines had an IOMMU.

But the qdev name of the device is still "iommu" and we can't change
that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
in amd_iommu.c?

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Blue Swirl]

On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  AMD IOMMU, which could lead to confusion unless we rename the former.

I was also thinking of renaming the file some time ago. The correct
name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
several IO-UNITs instead. All Sun4m machines had an IOMMU.

But the qdev name of the device is still "iommu" and we can't change
that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
in amd_iommu.c?

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Joerg Roedel]

On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  AMD IOMMU, which could lead to confusion unless we rename the former.
> 
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
> 
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

Keeping the plain name 'iommu' will likely cause confusion when more
iommu implementations are added. It is better to rename it so that the
name better describes what the file implements. So this change makes
sense for me.

	Joerg

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Joerg Roedel]

On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  AMD IOMMU, which could lead to confusion unless we rename the former.
> 
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
> 
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

Keeping the plain name 'iommu' will likely cause confusion when more
iommu implementations are added. It is better to rename it so that the
name better describes what the file implements. So this change makes
sense for me.

	Joerg

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Blue Swirl]

On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
>  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
>  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
>  >
>  > I was also thinking of renaming the file some time ago. The correct
>  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
>  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
>  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
>  >
>  > But the qdev name of the device is still "iommu" and we can't change
>  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
>  > in amd_iommu.c?
>
>
> Keeping the plain name 'iommu' will likely cause confusion when more
>  iommu implementations are added. It is better to rename it so that the
>  name better describes what the file implements. So this change makes
>  sense for me.

I see. I'm OK then with "sun4m_iommu.c".

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Blue Swirl]

On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
>  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
>  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
>  >
>  > I was also thinking of renaming the file some time ago. The correct
>  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
>  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
>  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
>  >
>  > But the qdev name of the device is still "iommu" and we can't change
>  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
>  > in amd_iommu.c?
>
>
> Keeping the plain name 'iommu' will likely cause confusion when more
>  iommu implementations are added. It is better to rename it so that the
>  name better describes what the file implements. So this change makes
>  sense for me.

I see. I'm OK then with "sun4m_iommu.c".

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Eduard - Gabriel Munteanu]

On Tue, Mar 30, 2010 at 11:00:10PM +0300, Blue Swirl wrote:
> On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> > On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> >  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> >  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
> >  >
> >  > I was also thinking of renaming the file some time ago. The correct
> >  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> >  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> >  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
> >  >
> >  > But the qdev name of the device is still "iommu" and we can't change
> >  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> >  > in amd_iommu.c?
> >
> >
> > Keeping the plain name 'iommu' will likely cause confusion when more
> >  iommu implementations are added. It is better to rename it so that the
> >  name better describes what the file implements. So this change makes
> >  sense for me.
> 
> I see. I'm OK then with "sun4m_iommu.c".

Yes, I think it's enough to just change the filename, since multiple
such devices aren't likely to conflict, in any configuration.

Then "sun4m_iommu.c" it is. Will resubmit with the next patchset.


	Eduard

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Eduard - Gabriel Munteanu]

On Tue, Mar 30, 2010 at 11:00:10PM +0300, Blue Swirl wrote:
> On 3/30/10, Joerg Roedel <joro@8bytes.org> wrote:
> > On Tue, Mar 30, 2010 at 08:06:36PM +0300, Blue Swirl wrote:
> >  > On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> >  > > hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
> >  > >  AMD IOMMU, which could lead to confusion unless we rename the former.
> >  >
> >  > I was also thinking of renaming the file some time ago. The correct
> >  > name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> >  > different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> >  > several IO-UNITs instead. All Sun4m machines had an IOMMU.
> >  >
> >  > But the qdev name of the device is still "iommu" and we can't change
> >  > that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> >  > in amd_iommu.c?
> >
> >
> > Keeping the plain name 'iommu' will likely cause confusion when more
> >  iommu implementations are added. It is better to rename it so that the
> >  name better describes what the file implements. So this change makes
> >  sense for me.
> 
> I see. I'm OK then with "sun4m_iommu.c".

Yes, I think it's enough to just change the filename, since multiple
such devices aren't likely to conflict, in any configuration.

Then "sun4m_iommu.c" it is. Will resubmit with the next patchset.


	Eduard

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Gerd Hoffmann]

On 03/30/10 19:06, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>  wrote:
>> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>>   AMD IOMMU, which could lead to confusion unless we rename the former.
>
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
>
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

I'd go for the (filename) rename.  The qdev name shouldn't cause 
conflicts due to the different targets.

cheers,
   Gerd

Re: [Qemu-devel] [RFC PATCH 4/7] sparc: rename hw/iommu.c

[Gerd Hoffmann]

On 03/30/10 19:06, Blue Swirl wrote:
> On 3/30/10, Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>  wrote:
>> hw/iommu.c concerns the SPARC IOMMU. However we intend to implement the
>>   AMD IOMMU, which could lead to confusion unless we rename the former.
>
> I was also thinking of renaming the file some time ago. The correct
> name would be "sun4m_iommu.c". Sun4c (while still Sparc based) had a
> different architecture (IIRC CPU MMU doubled as IOMMU) and Sun4d had
> several IO-UNITs instead. All Sun4m machines had an IOMMU.
>
> But the qdev name of the device is still "iommu" and we can't change
> that. So I'm not so sure it's worth renaming. Can't AMD IOMMU reside
> in amd_iommu.c?

I'd go for the (filename) rename.  The qdev name shouldn't cause 
conflicts due to the different targets.

cheers,
   Gerd

[RFC PATCH 5/7] x86-64: AMD IOMMU stub

[Eduard - Gabriel Munteanu]

This currently loads a non-functional IVRS ACPI table and provides a
skeleton for initializing the AMD IOMMU.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Makefile.target |    1 +
 hw/amd_iommu.c  |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c         |    2 +
 hw/pc.h         |    3 ++
 4 files changed, 109 insertions(+), 0 deletions(-)
 create mode 100644 hw/amd_iommu.c

diff --git a/Makefile.target b/Makefile.target
index cbe19a6..dfa4652 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -230,6 +230,7 @@ obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += extboot.o
 obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
 obj-i386-y += testdev.o
+obj-i386-y += amd_iommu.o
 
 obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c
new file mode 100644
index 0000000..b502430
--- /dev/null
+++ b/hw/amd_iommu.c
@@ -0,0 +1,103 @@
+/*
+ * AMD IOMMU emulation
+ *
+ * Copyright (c) 2010 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * IVRS (I/O Virtualization Reporting Structure) table.
+ *
+ * Describes the AMD IOMMU, as per:
+ * "AMD I/O Virtualization Technology (IOMMU) Specification", rev 1.26
+ */
+
+#include <stdint.h>
+
+#include "pc.h"
+
+struct ivrs_ivhd
+{
+    uint8_t    type;
+    uint8_t    flags;
+    uint16_t   length;
+    uint16_t   devid;
+    uint16_t   capab_off;
+    uint64_t   iommu_base_addr;
+    uint16_t   pci_seg_group;
+    uint16_t   iommu_info;
+    uint32_t   reserved;
+    uint32_t   entry;
+} __attribute__ ((__packed__));
+
+struct ivrs_table
+{
+    struct acpi_table_header    acpi_hdr;
+    uint32_t                    iv_info;
+    uint32_t                    reserved[2];
+    struct ivrs_ivhd            ivhd;
+} __attribute__ ((__packed__));
+
+static const char ivrs_sig[]    = "IVRS";
+static const char dfl_id[]      = "QEMUQEMU";
+
+static void amd_iommu_init_ivrs(void)
+{
+    int ivrs_size = sizeof(struct ivrs_table);
+    struct ivrs_table *ivrs;
+    struct ivrs_ivhd *ivhd;
+    struct acpi_table_header *acpi_hdr;
+
+    ivrs = acpi_alloc_table(ivrs_size);
+    acpi_hdr = &ivrs->acpi_hdr;
+    ivhd = &ivrs->ivhd;
+
+    ivrs->iv_info = (64 << 15) |    /* Virtual address space size. */
+                    (48 << 8);      /* Physical address space size. */
+
+    ivhd->type              = 0x10;
+    ivhd->flags             = 0;
+    ivhd->length            = sizeof(struct ivrs_ivhd);
+    ivhd->devid             = 0;
+    ivhd->capab_off         = 0;
+    ivhd->iommu_base_addr   = 0;
+    ivhd->pci_seg_group     = 0;
+    ivhd->iommu_info        = 0;
+    ivhd->reserved          = 0;
+    ivhd->entry             = 0;
+
+    strncpy(acpi_hdr->signature, ivrs_sig, 4);
+    acpi_hdr->revision = 1;
+    strncpy(acpi_hdr->oem_id, dfl_id, 6);
+    strncpy(acpi_hdr->oem_table_id, dfl_id, 8);
+    acpi_hdr->oem_revision = 1;
+    strncpy(acpi_hdr->asl_compiler_id, dfl_id, 4);
+    acpi_hdr->asl_compiler_revision = 1;
+
+    acpi_commit_table(ivrs);
+}
+
+int amd_iommu_init(void)
+{
+    amd_iommu_init_ivrs();
+
+    return 0;
+}
+
diff --git a/hw/pc.c b/hw/pc.c
index 0aebae9..89a7a30 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -906,6 +906,8 @@ static void pc_init1(ram_addr_t ram_size,
     cpu_register_physical_memory((uint32_t)(-bios_size),
                                  bios_size, bios_offset | IO_MEM_ROM);
 
+    amd_iommu_init();
+
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
diff --git a/hw/pc.h b/hw/pc.h
index 92954db..b91300e 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -194,4 +194,7 @@ int cpu_is_bsp(CPUState *env);
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 
+/* amd_iommu.c */
+int amd_iommu_init(void);
+
 #endif
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 5/7] x86-64: AMD IOMMU stub

[Eduard - Gabriel Munteanu]

This currently loads a non-functional IVRS ACPI table and provides a
skeleton for initializing the AMD IOMMU.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 Makefile.target |    1 +
 hw/amd_iommu.c  |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pc.c         |    2 +
 hw/pc.h         |    3 ++
 4 files changed, 109 insertions(+), 0 deletions(-)
 create mode 100644 hw/amd_iommu.c

diff --git a/Makefile.target b/Makefile.target
index cbe19a6..dfa4652 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -230,6 +230,7 @@ obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += extboot.o
 obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
 obj-i386-y += testdev.o
+obj-i386-y += amd_iommu.o
 
 obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c
new file mode 100644
index 0000000..b502430
--- /dev/null
+++ b/hw/amd_iommu.c
@@ -0,0 +1,103 @@
+/*
+ * AMD IOMMU emulation
+ *
+ * Copyright (c) 2010 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * IVRS (I/O Virtualization Reporting Structure) table.
+ *
+ * Describes the AMD IOMMU, as per:
+ * "AMD I/O Virtualization Technology (IOMMU) Specification", rev 1.26
+ */
+
+#include <stdint.h>
+
+#include "pc.h"
+
+struct ivrs_ivhd
+{
+    uint8_t    type;
+    uint8_t    flags;
+    uint16_t   length;
+    uint16_t   devid;
+    uint16_t   capab_off;
+    uint64_t   iommu_base_addr;
+    uint16_t   pci_seg_group;
+    uint16_t   iommu_info;
+    uint32_t   reserved;
+    uint32_t   entry;
+} __attribute__ ((__packed__));
+
+struct ivrs_table
+{
+    struct acpi_table_header    acpi_hdr;
+    uint32_t                    iv_info;
+    uint32_t                    reserved[2];
+    struct ivrs_ivhd            ivhd;
+} __attribute__ ((__packed__));
+
+static const char ivrs_sig[]    = "IVRS";
+static const char dfl_id[]      = "QEMUQEMU";
+
+static void amd_iommu_init_ivrs(void)
+{
+    int ivrs_size = sizeof(struct ivrs_table);
+    struct ivrs_table *ivrs;
+    struct ivrs_ivhd *ivhd;
+    struct acpi_table_header *acpi_hdr;
+
+    ivrs = acpi_alloc_table(ivrs_size);
+    acpi_hdr = &ivrs->acpi_hdr;
+    ivhd = &ivrs->ivhd;
+
+    ivrs->iv_info = (64 << 15) |    /* Virtual address space size. */
+                    (48 << 8);      /* Physical address space size. */
+
+    ivhd->type              = 0x10;
+    ivhd->flags             = 0;
+    ivhd->length            = sizeof(struct ivrs_ivhd);
+    ivhd->devid             = 0;
+    ivhd->capab_off         = 0;
+    ivhd->iommu_base_addr   = 0;
+    ivhd->pci_seg_group     = 0;
+    ivhd->iommu_info        = 0;
+    ivhd->reserved          = 0;
+    ivhd->entry             = 0;
+
+    strncpy(acpi_hdr->signature, ivrs_sig, 4);
+    acpi_hdr->revision = 1;
+    strncpy(acpi_hdr->oem_id, dfl_id, 6);
+    strncpy(acpi_hdr->oem_table_id, dfl_id, 8);
+    acpi_hdr->oem_revision = 1;
+    strncpy(acpi_hdr->asl_compiler_id, dfl_id, 4);
+    acpi_hdr->asl_compiler_revision = 1;
+
+    acpi_commit_table(ivrs);
+}
+
+int amd_iommu_init(void)
+{
+    amd_iommu_init_ivrs();
+
+    return 0;
+}
+
diff --git a/hw/pc.c b/hw/pc.c
index 0aebae9..89a7a30 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -906,6 +906,8 @@ static void pc_init1(ram_addr_t ram_size,
     cpu_register_physical_memory((uint32_t)(-bios_size),
                                  bios_size, bios_offset | IO_MEM_ROM);
 
+    amd_iommu_init();
+
     fw_cfg = bochs_bios_init();
     rom_set_fw(fw_cfg);
 
diff --git a/hw/pc.h b/hw/pc.h
index 92954db..b91300e 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -194,4 +194,7 @@ int cpu_is_bsp(CPUState *env);
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 
+/* amd_iommu.c */
+int amd_iommu_init(void);
+
 #endif
-- 
1.6.4.4

Re: [RFC PATCH 5/7] x86-64: AMD IOMMU stub

[Blue Swirl]

On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> This currently loads a non-functional IVRS ACPI table and provides a
>  skeleton for initializing the AMD IOMMU.
>
>  Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
>  ---
>   Makefile.target |    1 +
>   hw/amd_iommu.c  |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/pc.c         |    2 +
>   hw/pc.h         |    3 ++
>   4 files changed, 109 insertions(+), 0 deletions(-)
>   create mode 100644 hw/amd_iommu.c
>
>  diff --git a/Makefile.target b/Makefile.target
>  index cbe19a6..dfa4652 100644
>  --- a/Makefile.target
>  +++ b/Makefile.target
>  @@ -230,6 +230,7 @@ obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>   obj-i386-y += extboot.o
>   obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
>   obj-i386-y += testdev.o
>  +obj-i386-y += amd_iommu.o
>
>   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
>   obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
>  diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c
>  new file mode 100644
>  index 0000000..b502430
>  --- /dev/null
>  +++ b/hw/amd_iommu.c
>  @@ -0,0 +1,103 @@
>  +/*
>  + * AMD IOMMU emulation
>  + *
>  + * Copyright (c) 2010 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
>  + *
>  + * Permission is hereby granted, free of charge, to any person obtaining a copy
>  + * of this software and associated documentation files (the "Software"), to deal
>  + * in the Software without restriction, including without limitation the rights
>  + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>  + * copies of the Software, and to permit persons to whom the Software is
>  + * furnished to do so, subject to the following conditions:
>  + *
>  + * The above copyright notice and this permission notice shall be included in
>  + * all copies or substantial portions of the Software.
>  + *
>  + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>  + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>  + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>  + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>  + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>  + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>  + * THE SOFTWARE.
>  + */
>  +
>  +/*
>  + * IVRS (I/O Virtualization Reporting Structure) table.
>  + *
>  + * Describes the AMD IOMMU, as per:
>  + * "AMD I/O Virtualization Technology (IOMMU) Specification", rev 1.26
>  + */
>  +
>  +#include <stdint.h>
>  +
>  +#include "pc.h"
>  +
>  +struct ivrs_ivhd
>  +{
>  +    uint8_t    type;
>  +    uint8_t    flags;
>  +    uint16_t   length;
>  +    uint16_t   devid;
>  +    uint16_t   capab_off;
>  +    uint64_t   iommu_base_addr;
>  +    uint16_t   pci_seg_group;
>  +    uint16_t   iommu_info;
>  +    uint32_t   reserved;
>  +    uint32_t   entry;
>  +} __attribute__ ((__packed__));
>  +
>  +struct ivrs_table
>  +{
>  +    struct acpi_table_header    acpi_hdr;
>  +    uint32_t                    iv_info;
>  +    uint32_t                    reserved[2];
>  +    struct ivrs_ivhd            ivhd;
>  +} __attribute__ ((__packed__));
>  +
>  +static const char ivrs_sig[]    = "IVRS";
>  +static const char dfl_id[]      = "QEMUQEMU";
>  +
>  +static void amd_iommu_init_ivrs(void)
>  +{
>  +    int ivrs_size = sizeof(struct ivrs_table);
>  +    struct ivrs_table *ivrs;
>  +    struct ivrs_ivhd *ivhd;
>  +    struct acpi_table_header *acpi_hdr;
>  +
>  +    ivrs = acpi_alloc_table(ivrs_size);
>  +    acpi_hdr = &ivrs->acpi_hdr;
>  +    ivhd = &ivrs->ivhd;
>  +
>  +    ivrs->iv_info = (64 << 15) |    /* Virtual address space size. */
>  +                    (48 << 8);      /* Physical address space size. */
>  +
>  +    ivhd->type              = 0x10;
>  +    ivhd->flags             = 0;
>  +    ivhd->length            = sizeof(struct ivrs_ivhd);
>  +    ivhd->devid             = 0;
>  +    ivhd->capab_off         = 0;
>  +    ivhd->iommu_base_addr   = 0;
>  +    ivhd->pci_seg_group     = 0;
>  +    ivhd->iommu_info        = 0;
>  +    ivhd->reserved          = 0;
>  +    ivhd->entry             = 0;

The table would be filled with wrong data on big endian host. Please
use cpu_to_le64/32/16.

>  +    strncpy(acpi_hdr->signature, ivrs_sig, 4);
>  +    acpi_hdr->revision = 1;
>  +    strncpy(acpi_hdr->oem_id, dfl_id, 6);
>  +    strncpy(acpi_hdr->oem_table_id, dfl_id, 8);
>  +    acpi_hdr->oem_revision = 1;
>  +    strncpy(acpi_hdr->asl_compiler_id, dfl_id, 4);
>  +    acpi_hdr->asl_compiler_revision = 1;
>  +
>  +    acpi_commit_table(ivrs);
>  +}
>  +
>  +int amd_iommu_init(void)
>  +{
>  +    amd_iommu_init_ivrs();
>  +
>  +    return 0;
>  +}
>  +
>  diff --git a/hw/pc.c b/hw/pc.c
>  index 0aebae9..89a7a30 100644
>  --- a/hw/pc.c
>  +++ b/hw/pc.c
>  @@ -906,6 +906,8 @@ static void pc_init1(ram_addr_t ram_size,
>      cpu_register_physical_memory((uint32_t)(-bios_size),
>                                   bios_size, bios_offset | IO_MEM_ROM);
>
>  +    amd_iommu_init();
>  +
>      fw_cfg = bochs_bios_init();
>      rom_set_fw(fw_cfg);
>
>  diff --git a/hw/pc.h b/hw/pc.h
>  index 92954db..b91300e 100644
>  --- a/hw/pc.h
>  +++ b/hw/pc.h
>  @@ -194,4 +194,7 @@ int cpu_is_bsp(CPUState *env);
>
>   int e820_add_entry(uint64_t, uint64_t, uint32_t);
>
>  +/* amd_iommu.c */
>  +int amd_iommu_init(void);
>  +
>   #endif
>
> --
>  1.6.4.4
>
>
>
>

Re: [Qemu-devel] [RFC PATCH 5/7] x86-64: AMD IOMMU stub

[Blue Swirl]

On 3/30/10, Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> wrote:
> This currently loads a non-functional IVRS ACPI table and provides a
>  skeleton for initializing the AMD IOMMU.
>
>  Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
>  ---
>   Makefile.target |    1 +
>   hw/amd_iommu.c  |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/pc.c         |    2 +
>   hw/pc.h         |    3 ++
>   4 files changed, 109 insertions(+), 0 deletions(-)
>   create mode 100644 hw/amd_iommu.c
>
>  diff --git a/Makefile.target b/Makefile.target
>  index cbe19a6..dfa4652 100644
>  --- a/Makefile.target
>  +++ b/Makefile.target
>  @@ -230,6 +230,7 @@ obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>   obj-i386-y += extboot.o
>   obj-i386-y += ne2000-isa.o debugcon.o multiboot.o
>   obj-i386-y += testdev.o
>  +obj-i386-y += amd_iommu.o
>
>   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
>   obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
>  diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c
>  new file mode 100644
>  index 0000000..b502430
>  --- /dev/null
>  +++ b/hw/amd_iommu.c
>  @@ -0,0 +1,103 @@
>  +/*
>  + * AMD IOMMU emulation
>  + *
>  + * Copyright (c) 2010 Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
>  + *
>  + * Permission is hereby granted, free of charge, to any person obtaining a copy
>  + * of this software and associated documentation files (the "Software"), to deal
>  + * in the Software without restriction, including without limitation the rights
>  + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>  + * copies of the Software, and to permit persons to whom the Software is
>  + * furnished to do so, subject to the following conditions:
>  + *
>  + * The above copyright notice and this permission notice shall be included in
>  + * all copies or substantial portions of the Software.
>  + *
>  + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>  + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>  + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>  + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>  + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>  + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>  + * THE SOFTWARE.
>  + */
>  +
>  +/*
>  + * IVRS (I/O Virtualization Reporting Structure) table.
>  + *
>  + * Describes the AMD IOMMU, as per:
>  + * "AMD I/O Virtualization Technology (IOMMU) Specification", rev 1.26
>  + */
>  +
>  +#include <stdint.h>
>  +
>  +#include "pc.h"
>  +
>  +struct ivrs_ivhd
>  +{
>  +    uint8_t    type;
>  +    uint8_t    flags;
>  +    uint16_t   length;
>  +    uint16_t   devid;
>  +    uint16_t   capab_off;
>  +    uint64_t   iommu_base_addr;
>  +    uint16_t   pci_seg_group;
>  +    uint16_t   iommu_info;
>  +    uint32_t   reserved;
>  +    uint32_t   entry;
>  +} __attribute__ ((__packed__));
>  +
>  +struct ivrs_table
>  +{
>  +    struct acpi_table_header    acpi_hdr;
>  +    uint32_t                    iv_info;
>  +    uint32_t                    reserved[2];
>  +    struct ivrs_ivhd            ivhd;
>  +} __attribute__ ((__packed__));
>  +
>  +static const char ivrs_sig[]    = "IVRS";
>  +static const char dfl_id[]      = "QEMUQEMU";
>  +
>  +static void amd_iommu_init_ivrs(void)
>  +{
>  +    int ivrs_size = sizeof(struct ivrs_table);
>  +    struct ivrs_table *ivrs;
>  +    struct ivrs_ivhd *ivhd;
>  +    struct acpi_table_header *acpi_hdr;
>  +
>  +    ivrs = acpi_alloc_table(ivrs_size);
>  +    acpi_hdr = &ivrs->acpi_hdr;
>  +    ivhd = &ivrs->ivhd;
>  +
>  +    ivrs->iv_info = (64 << 15) |    /* Virtual address space size. */
>  +                    (48 << 8);      /* Physical address space size. */
>  +
>  +    ivhd->type              = 0x10;
>  +    ivhd->flags             = 0;
>  +    ivhd->length            = sizeof(struct ivrs_ivhd);
>  +    ivhd->devid             = 0;
>  +    ivhd->capab_off         = 0;
>  +    ivhd->iommu_base_addr   = 0;
>  +    ivhd->pci_seg_group     = 0;
>  +    ivhd->iommu_info        = 0;
>  +    ivhd->reserved          = 0;
>  +    ivhd->entry             = 0;

The table would be filled with wrong data on big endian host. Please
use cpu_to_le64/32/16.

>  +    strncpy(acpi_hdr->signature, ivrs_sig, 4);
>  +    acpi_hdr->revision = 1;
>  +    strncpy(acpi_hdr->oem_id, dfl_id, 6);
>  +    strncpy(acpi_hdr->oem_table_id, dfl_id, 8);
>  +    acpi_hdr->oem_revision = 1;
>  +    strncpy(acpi_hdr->asl_compiler_id, dfl_id, 4);
>  +    acpi_hdr->asl_compiler_revision = 1;
>  +
>  +    acpi_commit_table(ivrs);
>  +}
>  +
>  +int amd_iommu_init(void)
>  +{
>  +    amd_iommu_init_ivrs();
>  +
>  +    return 0;
>  +}
>  +
>  diff --git a/hw/pc.c b/hw/pc.c
>  index 0aebae9..89a7a30 100644
>  --- a/hw/pc.c
>  +++ b/hw/pc.c
>  @@ -906,6 +906,8 @@ static void pc_init1(ram_addr_t ram_size,
>      cpu_register_physical_memory((uint32_t)(-bios_size),
>                                   bios_size, bios_offset | IO_MEM_ROM);
>
>  +    amd_iommu_init();
>  +
>      fw_cfg = bochs_bios_init();
>      rom_set_fw(fw_cfg);
>
>  diff --git a/hw/pc.h b/hw/pc.h
>  index 92954db..b91300e 100644
>  --- a/hw/pc.h
>  +++ b/hw/pc.h
>  @@ -194,4 +194,7 @@ int cpu_is_bsp(CPUState *env);
>
>   int e820_add_entry(uint64_t, uint64_t, uint32_t);
>
>  +/* amd_iommu.c */
>  +int amd_iommu_init(void);
>  +
>   #endif
>
> --
>  1.6.4.4
>
>
>
>

[RFC PATCH 6/7] acpi: cleanup acpi_checksum()

[Eduard - Gabriel Munteanu]

This adds newlines in acpi_checksum() to separate the declarations, the
body and the return statement.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 3794f70..f067f85 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -832,9 +832,11 @@ size_t acpi_tables_len, acpi_tables_prev_len;
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
+
     sum = 0;
     for(i = 0; i < len; i++)
         sum += data[i];
+
     return (-sum) & 0xff;
 }
 
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 6/7] acpi: cleanup acpi_checksum()

[Eduard - Gabriel Munteanu]

This adds newlines in acpi_checksum() to separate the declarations, the
body and the return statement.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 3794f70..f067f85 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -832,9 +832,11 @@ size_t acpi_tables_len, acpi_tables_prev_len;
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
+
     sum = 0;
     for(i = 0; i < len; i++)
         sum += data[i];
+
     return (-sum) & 0xff;
 }
 
-- 
1.6.4.4

[RFC PATCH 7/7] acpi: fix bug in acpi_checksum() caused by garbage in checksum field

[Eduard - Gabriel Munteanu]

The whole table must sum to zero. We need to ignore garbage in the
checksum field (i.e. consider it zero) when checksumming. It is
legitimate to have garbage there, as the checksum makes sense only when
the table has been filled.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index f067f85..bb015f3 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -832,11 +832,16 @@ size_t acpi_tables_len, acpi_tables_prev_len;
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
+    struct acpi_table_header *acpi_hdr;
 
     sum = 0;
     for(i = 0; i < len; i++)
         sum += data[i];
 
+    /* Ignore preexisting garbage in checksum. */
+    acpi_hdr = (struct acpi_table_header *) data;
+    sum -= acpi_hdr->checksum;
+
     return (-sum) & 0xff;
 }
 
-- 
1.6.4.4

[Qemu-devel] [RFC PATCH 7/7] acpi: fix bug in acpi_checksum() caused by garbage in checksum field

[Eduard - Gabriel Munteanu]

The whole table must sum to zero. We need to ignore garbage in the
checksum field (i.e. consider it zero) when checksumming. It is
legitimate to have garbage there, as the checksum makes sense only when
the table has been filled.

Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
---
 hw/acpi.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index f067f85..bb015f3 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -832,11 +832,16 @@ size_t acpi_tables_len, acpi_tables_prev_len;
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
+    struct acpi_table_header *acpi_hdr;
 
     sum = 0;
     for(i = 0; i < len; i++)
         sum += data[i];
 
+    /* Ignore preexisting garbage in checksum. */
+    acpi_hdr = (struct acpi_table_header *) data;
+    sum -= acpi_hdr->checksum;
+
     return (-sum) & 0xff;
 }
 
-- 
1.6.4.4

Re: [Qemu-devel] [RFC PATCH 7/7] acpi: fix bug in acpi_checksum() caused by garbage in checksum field

[Richard Henderson]

On 03/30/2010 01:20 AM, Eduard - Gabriel Munteanu wrote:
> +    /* Ignore preexisting garbage in checksum. */
> +    acpi_hdr = (struct acpi_table_header *) data;
> +    sum -= acpi_hdr->checksum;
> +
>      return (-sum) & 0xff;

Wouldn't it be cleaner to adjust the acpi_checksum definition to take
and acpi_table_header operand instead of uint8_t?  And given it's only
usage, perhaps update the checksum instead of returning it?  E.g.

-    ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
+    acpi_update_checksum((struct acpi_table_header *)p, off);


r~

Re: [Qemu-devel] [RFC PATCH 7/7] acpi: fix bug in acpi_checksum() caused by garbage in checksum field

[Richard Henderson]

On 03/30/2010 01:20 AM, Eduard - Gabriel Munteanu wrote:
> +    /* Ignore preexisting garbage in checksum. */
> +    acpi_hdr = (struct acpi_table_header *) data;
> +    sum -= acpi_hdr->checksum;
> +
>      return (-sum) & 0xff;

Wouldn't it be cleaner to adjust the acpi_checksum definition to take
and acpi_table_header operand instead of uint8_t?  And given it's only
usage, perhaps update the checksum instead of returning it?  E.g.

-    ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
+    acpi_update_checksum((struct acpi_table_header *)p, off);


r~

Re: [RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Joerg Roedel]

Hello Eduard,

On Tue, Mar 30, 2010 at 11:20:01AM +0300, Eduard - Gabriel Munteanu wrote:
> This patchset is intended to provide a start for implementing the
> emulation of the AMD IOMMU. For those who aren't aware yet, I intend
> to participate as a student in GSoC 2010.

Great. This is a good start.

> In short, this demonstrates a mechanism of inserting ACPI tables without
> modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
> equivalent, but I think this approach is better, at least at the moment.

I like the approach implemented in this patchset because of its
simplicity. The right place for building acpi tables is the bios,
though. I am fine with both ways. Anthony, Avi, what do you
think about it?

> I wouldn't merge this patchset yet, at least stuff after the first patch,
> until it accumulates more work. I also didn't test loading ACPI tables from
> the command line after these modifications.

When Linux finds an IVRS table it expects that there is a working AMD
IOMMU so you should change this patchset so that the ACPI table is
enabled later when the hardware emulation is working. That will keep
this work bisectable.
As the next step I suggest you to implement an AMD IOMMU pci device
with the config space, capability and the mmio register space.

Regards,

	Joerg

[Qemu-devel] Re: [RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Joerg Roedel]

Hello Eduard,

On Tue, Mar 30, 2010 at 11:20:01AM +0300, Eduard - Gabriel Munteanu wrote:
> This patchset is intended to provide a start for implementing the
> emulation of the AMD IOMMU. For those who aren't aware yet, I intend
> to participate as a student in GSoC 2010.

Great. This is a good start.

> In short, this demonstrates a mechanism of inserting ACPI tables without
> modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
> equivalent, but I think this approach is better, at least at the moment.

I like the approach implemented in this patchset because of its
simplicity. The right place for building acpi tables is the bios,
though. I am fine with both ways. Anthony, Avi, what do you
think about it?

> I wouldn't merge this patchset yet, at least stuff after the first patch,
> until it accumulates more work. I also didn't test loading ACPI tables from
> the command line after these modifications.

When Linux finds an IVRS table it expects that there is a working AMD
IOMMU so you should change this patchset so that the ACPI table is
enabled later when the hardware emulation is working. That will keep
this work bisectable.
As the next step I suggest you to implement an AMD IOMMU pci device
with the config space, capability and the mmio register space.

Regards,

	Joerg

Re: [RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Avi Kivity]

On 03/30/2010 10:40 PM, Joerg Roedel wrote:
>
>> In short, this demonstrates a mechanism of inserting ACPI tables without
>> modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
>> equivalent, but I think this approach is better, at least at the moment.
>>      
> I like the approach implemented in this patchset because of its
> simplicity. The right place for building acpi tables is the bios,
> though. I am fine with both ways. Anthony, Avi, what do you
> think about it?
>    

I agree.  Let qemu emulate the hardware, build the tables in the BIOS.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

[Qemu-devel] Re: [RFC PATCH 0/7] Beginning implementing the AMD IOMMU emulation

[Avi Kivity]

On 03/30/2010 10:40 PM, Joerg Roedel wrote:
>
>> In short, this demonstrates a mechanism of inserting ACPI tables without
>> modifying SeaBIOS or other BIOS implementations. I also have a SeaBIOS
>> equivalent, but I think this approach is better, at least at the moment.
>>      
> I like the approach implemented in this patchset because of its
> simplicity. The right place for building acpi tables is the bios,
> though. I am fine with both ways. Anthony, Avi, what do you
> think about it?
>    

I agree.  Let qemu emulate the hardware, build the tables in the BIOS.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.