[Qemu-devel] [Patch 0/12] Pciinit redesign and 64bit PCI support
This patch series redesigns and adds new features to pciinit.c code. [Patches 1-4] There are no more arrays of bases and counts in new implementation. The new implementation is based on dynamic allocation of pci_region_entry structures which are grouped into lists. Each pci_region_entry structure could be a PCI bar or a downstream PCI region (bridge). Each entry has a set of attributes: type (IO, MEM,PREFMEM), is64bit, size. [Patch 5] Bridge regions are no longer rounded up to the next highest size - instead track alignment explicitly. This should improve the memory layout for bridge regions. Also, unused bridge regions will no longer be allocated any space. [Patch 6] Patch takes into account PCI bar and ROM regions of PCI bridges [Patches 7-12] Make pciinit.c 64bit aware. Support discovery and configuration of 64bit bars, with non-zero upper32 bits. Code allocates the 64bit PCI bars in high address range if they don't fir in 32bit range. Kevin O'Konor (6): 0001-pciinit-Introduction-of-pci_region_entry-structure.patch 0002-pciinit-Move-bus-bar-asignment.patch 0004-pciinit-Use-sorted-order-allocation.patch 0005-pciinit-Track-region-alignment-explicitly.patch 0006-pciinit-bridges-can-have-two-regions-too.patch 0007-pciinit-Switch-to-64bit-variable-types.patch Alexey Korolev (6): 0003-pciinit-Remove-size-element-from-pci_bus-r-structure.patch 0008-pciinit-Add-pci_region-structure.patch 0009-pciinit-64bit-capability-discovery-for-pci-bridges.patch 0010-Do-not-store-pci-region-stats-instead-calulate-the-s.patch 0011-Migrate-64bit-entries-to-64bit-pci-regions.patch 0012-Fix-64bit-PCI-issues-on-Windows.patch src/acpi-dsdt.dsl |7 + src/acpi-dsdt.hex | 64 +++- src/config.h |2 + src/pci.h |6 +- src/pciinit.c | 464 ++--- 5 files changed, 325 insertions(+), 218 deletions(-)
[Qemu-devel] [PATCH 01/12] pciinit: Introduction of pci_region_entry structure
The pci_region_entry structure is introduced.
The pci_device-bars are removed. The information from
pci_region_entry is used to program pci bars.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pci.h |5 --
src/pciinit.c | 116 +++-
2 files changed, 81 insertions(+), 40 deletions(-)
diff --git a/src/pci.h b/src/pci.h
index a2a5a4c..5598100 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -51,11 +51,6 @@ struct pci_device {
u8 prog_if, revision;
u8 header_type;
u8 secondary_bus;
-struct {
-u32 addr;
-u32 size;
-int is64;
-} bars[PCI_NUM_REGIONS];
// Local information on device.
int have_driver;
diff --git a/src/pciinit.c b/src/pciinit.c
index 9f3fdd4..953f3bd 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -31,6 +31,15 @@ static const char *region_type_name[] = {
[ PCI_REGION_TYPE_PREFMEM ] = prefmem,
};
+struct pci_region_entry {
+struct pci_device *dev;
+int bar;
+u32 size;
+int is64;
+enum pci_region_type type;
+struct pci_region_entry *next;
+};
+
struct pci_bus {
struct {
/* pci region stats */
@@ -41,6 +50,7 @@ struct pci_bus {
/* pci region assignments */
u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
+struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
@@ -352,19 +362,41 @@ pci_bios_get_bar(struct pci_device *pci, int bar, u32
*val, u32 *size)
*size = (~(*val mask)) + 1;
}
-static void pci_bios_bus_reserve(struct pci_bus *bus, int type, u32 size)
+static struct pci_region_entry *
+pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
+int bar, u32 size, int type, int is64)
{
-u32 index;
+struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
+if (!entry) {
+warn_noalloc();
+return NULL;
+}
+memset(entry, 0, sizeof(*entry));
+entry-dev = dev;
+entry-bar = bar;
+entry-size = size;
+entry-is64 = is64;
+entry-type = type;
+// Insert into list in sorted order.
+struct pci_region_entry **pprev;
+for (pprev = bus-r[type].list; *pprev; pprev = (*pprev)-next) {
+struct pci_region_entry *pos = *pprev;
+if (pos-size size)
+break;
+}
+entry-next = *pprev;
+*pprev = entry;
-index = pci_size_to_index(size, type);
+int index = pci_size_to_index(size, type);
size = pci_index_to_size(index, type);
bus-r[type].count[index]++;
bus-r[type].sum += size;
if (bus-r[type].max size)
bus-r[type].max = size;
+return entry;
}
-static void pci_bios_check_devices(struct pci_bus *busses)
+static int pci_bios_check_devices(struct pci_bus *busses)
{
dprintf(1, PCI: check devices\n);
@@ -383,14 +415,15 @@ static void pci_bios_check_devices(struct pci_bus *busses)
if (val == 0)
continue;
-pci_bios_bus_reserve(bus, pci_addr_to_type(val), size);
-pci-bars[i].addr = val;
-pci-bars[i].size = size;
-pci-bars[i].is64 = (!(val PCI_BASE_ADDRESS_SPACE_IO)
- (val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
- == PCI_BASE_ADDRESS_MEM_TYPE_64);
+int is64 = (!(val PCI_BASE_ADDRESS_SPACE_IO)
+(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+== PCI_BASE_ADDRESS_MEM_TYPE_64);
+struct pci_region_entry *entry = pci_region_create_entry(
+bus, pci, i, size, pci_addr_to_type(val), is64);
+if (!entry)
+return -1;
-if (pci-bars[i].is64)
+if (is64)
i++;
}
}
@@ -410,7 +443,11 @@ static void pci_bios_check_devices(struct pci_bus *busses)
if (s-r[type].size limit)
s-r[type].size = limit;
s-r[type].size = pci_size_roundup(s-r[type].size);
-pci_bios_bus_reserve(parent, type, s-r[type].size);
+// entry-bar is -1 if the entry represents a bridge region
+struct pci_region_entry *entry = pci_region_create_entry(
+parent, s-bus_dev, -1, s-r[type].size, type, 0);
+if (!entry)
+return -1;
}
dprintf(1, PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n,
secondary_bus,
@@ -418,6 +455,7 @@ static void pci_bios_check_devices(struct pci_bus *busses)
s-r[PCI_REGION_TYPE_MEM].size,
s-r[PCI_REGION_TYPE_PREFMEM].size);
}
+return 0;
}
#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
@@ -483,6 +521,24 @@ static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int
type, u32 size)
#define PCI_MEMORY_SHIFT16
#define
[Qemu-devel] [PATCH 02/12] pciinit: Move bus bar asignment
Perform bus bar assignment at same time as normal bar assignment
Signed-off-by: Kevin O'Connor ke...@koconnor.net
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 53 ++---
1 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 953f3bd..74ade52 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -526,8 +526,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct
pci_region_entry *entry)
{
u16 bdf = entry-dev-bdf;
struct pci_bus *bus = busses[pci_bdf_to_bus(bdf)];
+u32 addr = pci_bios_bus_get_addr(bus, entry-type, entry-size);
if (entry-bar = 0) {
-u32 addr = pci_bios_bus_get_addr(bus, entry-type, entry-size);
dprintf(1, PCI: map device bdf=%02x:%02x.%x
bar %d, addr %08x, size %08x [%s]\n,
pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf),
@@ -536,54 +536,37 @@ pci_region_map_one_entry(struct pci_bus *busses, struct
pci_region_entry *entry)
pci_set_io_region_addr(entry-dev, entry-bar, addr);
if (entry-is64)
pci_set_io_region_addr(entry-dev, entry-bar + 1, 0);
+return;
}
-}
-
-static void pci_bios_map_devices(struct pci_bus *busses)
-{
-// Setup bases for root bus.
-dprintf(1, PCI: init bases bus 0 (primary)\n);
-pci_bios_init_bus_bases(busses[0]);
-
-// Map regions on each secondary bus.
-int secondary_bus;
-for (secondary_bus=1; secondary_bus=MaxPCIBus; secondary_bus++) {
-struct pci_bus *s = busses[secondary_bus];
-if (!s-bus_dev)
-continue;
-u16 bdf = s-bus_dev-bdf;
-struct pci_bus *parent = busses[pci_bdf_to_bus(bdf)];
-int type;
-for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-s-r[type].base = pci_bios_bus_get_addr(
-parent, type, s-r[type].size);
-}
-dprintf(1, PCI: init bases bus %d (secondary)\n, secondary_bus);
-pci_bios_init_bus_bases(s);
-u32 base = s-r[PCI_REGION_TYPE_IO].base;
-u32 limit = base + s-r[PCI_REGION_TYPE_IO].size - 1;
-pci_config_writeb(bdf, PCI_IO_BASE, base PCI_IO_SHIFT);
+struct pci_bus *child_bus = busses[entry-dev-secondary_bus];
+child_bus-r[entry-type].base = addr;
+u32 limit = addr + entry-size - 1;
+if (entry-type == PCI_REGION_TYPE_IO) {
+pci_config_writeb(bdf, PCI_IO_BASE, addr PCI_IO_SHIFT);
pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
pci_config_writeb(bdf, PCI_IO_LIMIT, limit PCI_IO_SHIFT);
pci_config_writew(bdf, PCI_IO_LIMIT_UPPER16, 0);
-
-base = s-r[PCI_REGION_TYPE_MEM].base;
-limit = base + s-r[PCI_REGION_TYPE_MEM].size - 1;
-pci_config_writew(bdf, PCI_MEMORY_BASE, base PCI_MEMORY_SHIFT);
+}
+if (entry-type == PCI_REGION_TYPE_MEM) {
+pci_config_writew(bdf, PCI_MEMORY_BASE, addr PCI_MEMORY_SHIFT);
pci_config_writew(bdf, PCI_MEMORY_LIMIT, limit PCI_MEMORY_SHIFT);
-
-base = s-r[PCI_REGION_TYPE_PREFMEM].base;
-limit = base + s-r[PCI_REGION_TYPE_PREFMEM].size - 1;
-pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, base
PCI_PREF_MEMORY_SHIFT);
+}
+if (entry-type == PCI_REGION_TYPE_PREFMEM) {
+pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, addr
PCI_PREF_MEMORY_SHIFT);
pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit
PCI_PREF_MEMORY_SHIFT);
pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0);
pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0);
}
+}
+static void pci_bios_map_devices(struct pci_bus *busses)
+{
// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
+dprintf(1, PCI: init bases bus %d\n, bus);
+pci_bios_init_bus_bases(busses[bus]);
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
struct pci_region_entry *entry = busses[bus].r[type].list;
--
1.7.5.4
[Qemu-devel] [PATCH 03/12] pciinit: Remove size element from pci_bus-r structure
The 'size' element of pci_bus-r structure is no
longer need as the information about bridge region
size is already stored in pci_region_entry structure.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pciinit.c | 20
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 74ade52..4617793 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -45,8 +45,6 @@ struct pci_bus {
/* pci region stats */
u32 count[32 - PCI_MEM_INDEX_SHIFT];
u32 sum, max;
-/* seconday bus region sizes */
-u32 size;
/* pci region assignments */
u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
@@ -439,21 +437,19 @@ static int pci_bios_check_devices(struct pci_bus *busses)
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
u32 limit = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-s-r[type].size = s-r[type].sum;
-if (s-r[type].size limit)
-s-r[type].size = limit;
-s-r[type].size = pci_size_roundup(s-r[type].size);
+u32 size = s-r[type].sum;
+if (size limit)
+size = limit;
+size = pci_size_roundup(size);
// entry-bar is -1 if the entry represents a bridge region
struct pci_region_entry *entry = pci_region_create_entry(
-parent, s-bus_dev, -1, s-r[type].size, type, 0);
+parent, s-bus_dev, -1, size, type, 0);
if (!entry)
return -1;
+dprintf(1, PCI: secondary bus %d size %x type %s\n,
+ entry-dev-secondary_bus, size,
+ region_type_name[entry-type]);
}
-dprintf(1, PCI: secondary bus %d sizes: io %x, mem %x, prefmem %x\n,
-secondary_bus,
-s-r[PCI_REGION_TYPE_IO].size,
-s-r[PCI_REGION_TYPE_MEM].size,
-s-r[PCI_REGION_TYPE_PREFMEM].size);
}
return 0;
}
--
1.7.5.4
[Qemu-devel] [PATCH 04/12] pciinit: Use sorted order allocation
Use sorted order allocation scheme instead of
array based count scheme.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pciinit.c | 71 +---
1 files changed, 7 insertions(+), 64 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 4617793..1b31177 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -12,9 +12,7 @@
#include pci_regs.h // PCI_COMMAND
#include xen.h // usingXen
-#define PCI_IO_INDEX_SHIFT 2
-#define PCI_MEM_INDEX_SHIFT 12
-
+#define PCI_DEVICE_MEM_MIN 0x1000
#define PCI_BRIDGE_IO_MIN 0x1000
#define PCI_BRIDGE_MEM_MIN 0x10
@@ -43,36 +41,14 @@ struct pci_region_entry {
struct pci_bus {
struct {
/* pci region stats */
-u32 count[32 - PCI_MEM_INDEX_SHIFT];
u32 sum, max;
/* pci region assignments */
-u32 bases[32 - PCI_MEM_INDEX_SHIFT];
u32 base;
struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
-static int pci_size_to_index(u32 size, enum pci_region_type type)
-{
-int index = __fls(size);
-int shift = (type == PCI_REGION_TYPE_IO) ?
-PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-if (index shift)
-index = shift;
-index -= shift;
-return index;
-}
-
-static u32 pci_index_to_size(int index, enum pci_region_type type)
-{
-int shift = (type == PCI_REGION_TYPE_IO) ?
-PCI_IO_INDEX_SHIFT : PCI_MEM_INDEX_SHIFT;
-
-return 0x1 (index + shift);
-}
-
static enum pci_region_type pci_addr_to_type(u32 addr)
{
if (addr PCI_BASE_ADDRESS_SPACE_IO)
@@ -385,9 +361,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
entry-next = *pprev;
*pprev = entry;
-int index = pci_size_to_index(size, type);
-size = pci_index_to_size(index, type);
-bus-r[type].count[index]++;
bus-r[type].sum += size;
if (bus-r[type].max size)
bus-r[type].max = size;
@@ -413,11 +386,14 @@ static int pci_bios_check_devices(struct pci_bus *busses)
if (val == 0)
continue;
+int type = pci_addr_to_type(val);
+if (type != PCI_REGION_TYPE_IO size PCI_DEVICE_MEM_MIN)
+size = PCI_DEVICE_MEM_MIN;
int is64 = (!(val PCI_BASE_ADDRESS_SPACE_IO)
(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
== PCI_BASE_ADDRESS_MEM_TYPE_64);
struct pci_region_entry *entry = pci_region_create_entry(
-bus, pci, i, size, pci_addr_to_type(val), is64);
+bus, pci, i, size, type, is64);
if (!entry)
return -1;
@@ -481,38 +457,6 @@ static int pci_bios_init_root_regions(struct pci_bus *bus,
u32 start, u32 end)
* BAR assignment
/
-static void pci_bios_init_bus_bases(struct pci_bus *bus)
-{
-u32 base, newbase, size;
-int type, i;
-
-for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-dprintf(1, type %s max %x sum %x base %x\n, region_type_name[type],
-bus-r[type].max, bus-r[type].sum, bus-r[type].base);
-base = bus-r[type].base;
-for (i = ARRAY_SIZE(bus-r[type].count)-1; i = 0; i--) {
-size = pci_index_to_size(i, type);
-if (!bus-r[type].count[i])
-continue;
-newbase = base + size * bus-r[type].count[i];
-dprintf(1, size %8x: %d bar(s), %8x - %8x\n,
-size, bus-r[type].count[i], base, newbase - 1);
-bus-r[type].bases[i] = base;
-base = newbase;
-}
-}
-}
-
-static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
-{
-u32 index, addr;
-
-index = pci_size_to_index(size, type);
-addr = bus-r[type].bases[index];
-bus-r[type].bases[index] += pci_index_to_size(index, type);
-return addr;
-}
-
#define PCI_IO_SHIFT8
#define PCI_MEMORY_SHIFT16
#define PCI_PREF_MEMORY_SHIFT 16
@@ -522,7 +466,8 @@ pci_region_map_one_entry(struct pci_bus *busses, struct
pci_region_entry *entry)
{
u16 bdf = entry-dev-bdf;
struct pci_bus *bus = busses[pci_bdf_to_bus(bdf)];
-u32 addr = pci_bios_bus_get_addr(bus, entry-type, entry-size);
+u32 addr = bus-r[entry-type].base;
+bus-r[entry-type].base += entry-size;
if (entry-bar = 0) {
dprintf(1, PCI: map device bdf=%02x:%02x.%x
bar %d, addr %08x, size %08x [%s]\n,
@@ -561,8 +506,6 @@ static void pci_bios_map_devices(struct pci_bus *busses)
// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
-dprintf(1, PCI: init bases bus %d\n, bus);
-pci_bios_init_bus_bases(busses[bus]);
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
[Qemu-devel] [PATCH 05/12] pciinit: Track region alignment explicitly.
Don't round up bridge regions to the next highest size - instead track
alignment explicitly. This should improve the memory layout for
bridge regions.
Also, unused bridge regions will no longer be allocated any space.
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pciinit.c | 41 ++---
1 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 1b31177..2bd4426 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -33,6 +33,7 @@ struct pci_region_entry {
struct pci_device *dev;
int bar;
u32 size;
+u32 align;
int is64;
enum pci_region_type type;
struct pci_region_entry *next;
@@ -41,7 +42,7 @@ struct pci_region_entry {
struct pci_bus {
struct {
/* pci region stats */
-u32 sum, max;
+u32 sum, align;
/* pci region assignments */
u32 base;
struct pci_region_entry *list;
@@ -307,12 +308,6 @@ pci_bios_init_bus(void)
* Bus sizing
/
-static u32 pci_size_roundup(u32 size)
-{
-int index = __fls(size-1)+1;
-return 0x1 index;
-}
-
static void
pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
{
@@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
u32 *val, u32 *size)
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
-int bar, u32 size, int type, int is64)
+int bar, u32 size, u32 align, int type, int
is64)
{
struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
if (!entry) {
@@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus,
struct pci_device *dev,
entry-dev = dev;
entry-bar = bar;
entry-size = size;
+entry-align = align;
entry-is64 = is64;
entry-type = type;
// Insert into list in sorted order.
struct pci_region_entry **pprev;
for (pprev = bus-r[type].list; *pprev; pprev = (*pprev)-next) {
struct pci_region_entry *pos = *pprev;
-if (pos-size size)
+if (pos-align align || (pos-align == align pos-size
size))
break;
}
entry-next = *pprev;
*pprev = entry;
bus-r[type].sum += size;
-if (bus-r[type].max size)
-bus-r[type].max = size;
+if (bus-r[type].align align)
+bus-r[type].align = align;
return entry;
}
@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
== PCI_BASE_ADDRESS_MEM_TYPE_64);
struct pci_region_entry *entry = pci_region_create_entry(
-bus, pci, i, size, type, is64);
+bus, pci, i, size, size, type, is64);
if (!entry)
return -1;
@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
struct pci_bus *parent =
busses[pci_bdf_to_bus(s-bus_dev-bdf)];
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-u32 limit = (type == PCI_REGION_TYPE_IO) ?
+u32 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-u32 size = s-r[type].sum;
-if (size limit)
-size = limit;
-size = pci_size_roundup(size);
+if (s-r[type].align align)
+align = s-r[type].align;
+u32 size = ALIGN(s-r[type].sum, align);
// entry-bar is -1 if the entry represents a bridge region
struct pci_region_entry *entry = pci_region_create_entry(
-parent, s-bus_dev, -1, size, type, 0);
+parent, s-bus_dev, -1, size, align, type, 0);
if (!entry)
return -1;
dprintf(1, PCI: secondary bus %d size %x type %s\n,
@@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
return 0;
}
-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
+#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment)
static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start,
u32 end)
@@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct
pci_bus *bus, u32 start, u32 end)
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-if (bus-r[reg1].sum bus-r[reg2].sum) {
-// Swap regions so larger area is more likely to align well.
+if (bus-r[reg1].align bus-r[reg2].align) {
+// Swap regions to improve alignment.
reg1 = PCI_REGION_TYPE_MEM;
reg2 = PCI_REGION_TYPE_PREFMEM;
}
-bus-r[reg2].base = ROOT_BASE(end, bus-r[reg2].sum,
bus-r[reg2].max);
+bus-r[reg2].base =
[Qemu-devel] [PATCH 06/12] pciinit: bridges can have two regions too
Patch takes into account PCI bar and ROM regions of PCI bridges
Original patch by: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pci.h |1 +
src/pciinit.c |8 +---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/pci.h b/src/pci.h
index 5598100..6be838c 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -5,6 +5,7 @@
#define PCI_ROM_SLOT 6
#define PCI_NUM_REGIONS 7
+#define PCI_BRIDGE_NUM_REGIONS 2
static inline u8 pci_bdf_to_bus(u16 bdf) {
return bdf 8;
diff --git a/src/pciinit.c b/src/pciinit.c
index 2bd4426..9b521e3 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -370,13 +370,15 @@ static int pci_bios_check_devices(struct pci_bus *busses)
// Calculate resources needed for regular (non-bus) devices.
struct pci_device *pci;
foreachpci(pci) {
-if (pci-class == PCI_CLASS_BRIDGE_PCI) {
+if (pci-class == PCI_CLASS_BRIDGE_PCI)
busses[pci-secondary_bus].bus_dev = pci;
-continue;
-}
+
struct pci_bus *bus = busses[pci_bdf_to_bus(pci-bdf)];
int i;
for (i = 0; i PCI_NUM_REGIONS; i++) {
+if ((pci-class == PCI_CLASS_BRIDGE_PCI)
+(i = PCI_BRIDGE_NUM_REGIONS i PCI_ROM_SLOT))
+continue;
u32 val, size;
pci_bios_get_bar(pci, i, val, size);
if (val == 0)
--
1.7.5.4
[Qemu-devel] [PATCH 07/12] pciinit: Switch to 64bit variable types.
Switch to 64bit variable types.
Add parsing 64bit bars.
Original patch by: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pciinit.c | 116 ++---
1 files changed, 61 insertions(+), 55 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 9b521e3..bc603c7 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -32,8 +32,8 @@ static const char *region_type_name[] = {
struct pci_region_entry {
struct pci_device *dev;
int bar;
-u32 size;
-u32 align;
+u64 size;
+u64 align;
int is64;
enum pci_region_type type;
struct pci_region_entry *next;
@@ -42,23 +42,14 @@ struct pci_region_entry {
struct pci_bus {
struct {
/* pci region stats */
-u32 sum, align;
+u64 sum, align;
/* pci region assignments */
-u32 base;
+u64 base;
struct pci_region_entry *list;
} r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
-static enum pci_region_type pci_addr_to_type(u32 addr)
-{
-if (addr PCI_BASE_ADDRESS_SPACE_IO)
-return PCI_REGION_TYPE_IO;
-if (addr PCI_BASE_ADDRESS_MEM_PREFETCH)
-return PCI_REGION_TYPE_PREFMEM;
-return PCI_REGION_TYPE_MEM;
-}
-
static u32 pci_bar(struct pci_device *pci, int region_num)
{
if (region_num != PCI_ROM_SLOT) {
@@ -71,9 +62,12 @@ static u32 pci_bar(struct pci_device *pci, int region_num)
}
static void
-pci_set_io_region_addr(struct pci_device *pci, int region_num, u32 addr)
+pci_set_io_region_addr(struct pci_device *pci, int bar, u64 addr, int is64)
{
-pci_config_writel(pci-bdf, pci_bar(pci, region_num), addr);
+u32 ofs = pci_bar(pci, bar);
+pci_config_writel(pci-bdf, ofs, addr);
+if (is64)
+pci_config_writel(pci-bdf, ofs + 4, addr 32);
}
@@ -126,10 +120,10 @@ static const struct pci_device_id pci_isa_bridge_tbl[] = {
static void storage_ide_init(struct pci_device *pci, void *arg)
{
/* IDE: we map it as in ISA mode */
-pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE);
-pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE);
-pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE);
-pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE);
+pci_set_io_region_addr(pci, 0, PORT_ATA1_CMD_BASE, 0);
+pci_set_io_region_addr(pci, 1, PORT_ATA1_CTRL_BASE, 0);
+pci_set_io_region_addr(pci, 2, PORT_ATA2_CMD_BASE, 0);
+pci_set_io_region_addr(pci, 3, PORT_ATA2_CTRL_BASE, 0);
}
/* PIIX3/PIIX4 IDE */
@@ -143,13 +137,13 @@ static void piix_ide_init(struct pci_device *pci, void
*arg)
static void pic_ibm_init(struct pci_device *pci, void *arg)
{
/* PIC, IBM, MPIC MPIC2 */
-pci_set_io_region_addr(pci, 0, 0x8080 + 0x0004);
+pci_set_io_region_addr(pci, 0, 0x8080 + 0x0004, 0);
}
static void apple_macio_init(struct pci_device *pci, void *arg)
{
/* macio bridge */
-pci_set_io_region_addr(pci, 0, 0x8080);
+pci_set_io_region_addr(pci, 0, 0x8080, 0);
}
static const struct pci_device_id pci_class_tbl[] = {
@@ -309,31 +303,51 @@ pci_bios_init_bus(void)
/
static void
-pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
+pci_bios_get_bar(struct pci_device *pci, int bar,
+ int *ptype, u64 *psize, int *pis64)
{
u32 ofs = pci_bar(pci, bar);
u16 bdf = pci-bdf;
u32 old = pci_config_readl(bdf, ofs);
-u32 mask;
+int is64 = 0, type = PCI_REGION_TYPE_MEM;
+u64 mask;
if (bar == PCI_ROM_SLOT) {
mask = PCI_ROM_ADDRESS_MASK;
pci_config_writel(bdf, ofs, mask);
} else {
-if (old PCI_BASE_ADDRESS_SPACE_IO)
+if (old PCI_BASE_ADDRESS_SPACE_IO) {
mask = PCI_BASE_ADDRESS_IO_MASK;
-else
+type = PCI_REGION_TYPE_IO;
+} else {
mask = PCI_BASE_ADDRESS_MEM_MASK;
+if (old PCI_BASE_ADDRESS_MEM_PREFETCH)
+type = PCI_REGION_TYPE_PREFMEM;
+is64 = ((old PCI_BASE_ADDRESS_MEM_TYPE_MASK)
+== PCI_BASE_ADDRESS_MEM_TYPE_64);
+}
pci_config_writel(bdf, ofs, ~0);
}
-*val = pci_config_readl(bdf, ofs);
+u64 val = pci_config_readl(bdf, ofs);
pci_config_writel(bdf, ofs, old);
-*size = (~(*val mask)) + 1;
+if (is64) {
+u32 hold = pci_config_readl(bdf, ofs + 4);
+pci_config_writel(bdf, ofs + 4, ~0);
+u32 high = pci_config_readl(bdf, ofs + 4);
+pci_config_writel(bdf, ofs + 4, hold);
+val |= ((u64)high 32);
+mask |= ((u64)0x 32);
+*psize = (~(val mask)) + 1;
+} else {
+*psize = ((~(val mask)) + 1) 0x;
+}
+*ptype = type;
+*pis64 = is64;
}
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
-
[Qemu-devel] [PATCH 08/12] pciinit: Add pci_region structure.
The pci_region structure is added.
Move setting of bus base address to pci_region_map_entries.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 50 --
1 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index bc603c7..f185cbd 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -39,14 +39,16 @@ struct pci_region_entry {
struct pci_region_entry *next;
};
+struct pci_region {
+/* pci region stats */
+u64 sum, align;
+/* pci region assignments */
+u64 base;
+struct pci_region_entry *list;
+};
+
struct pci_bus {
-struct {
-/* pci region stats */
-u64 sum, align;
-/* pci region assignments */
-u64 base;
-struct pci_region_entry *list;
-} r[PCI_REGION_TYPE_COUNT];
+struct pci_region r[PCI_REGION_TYPE_COUNT];
struct pci_device *bus_dev;
};
@@ -472,12 +474,9 @@ static void pci_bios_init_root_regions(struct pci_bus *bus)
#define PCI_PREF_MEMORY_SHIFT 16
static void
-pci_region_map_one_entry(struct pci_bus *busses, struct pci_region_entry
*entry)
+pci_region_map_one_entry(struct pci_region_entry *entry, u64 addr)
{
u16 bdf = entry-dev-bdf;
-struct pci_bus *bus = busses[pci_bdf_to_bus(bdf)];
-u64 addr = bus-r[entry-type].base;
-bus-r[entry-type].base += entry-size;
if (entry-bar = 0) {
dprintf(1, PCI: map device bdf=%02x:%02x.%x
bar %d, addr %08llx, size %08llx [%s]\n,
@@ -488,8 +487,6 @@ pci_region_map_one_entry(struct pci_bus *busses, struct
pci_region_entry *entry)
return;
}
-struct pci_bus *child_bus = busses[entry-dev-secondary_bus];
-child_bus-r[entry-type].base = addr;
u64 limit = addr + entry-size - 1;
if (entry-type == PCI_REGION_TYPE_IO) {
pci_config_writeb(bdf, PCI_IO_BASE, addr PCI_IO_SHIFT);
@@ -509,21 +506,30 @@ pci_region_map_one_entry(struct pci_bus *busses, struct
pci_region_entry *entry)
}
}
+static void pci_region_map_entries(struct pci_bus *busses, struct pci_region
*r)
+{
+struct pci_region_entry *entry = r-list;
+while(entry) {
+u64 addr = r-base;
+r-base += entry-size;
+if (entry-bar == -1)
+// Update bus base address if entry is a bridge region
+busses[entry-dev-secondary_bus].r[entry-type].base = addr;
+pci_region_map_one_entry(entry, addr);
+struct pci_region_entry *next = entry-next;
+free(entry);
+entry = next;
+}
+}
+
static void pci_bios_map_devices(struct pci_bus *busses)
{
// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
int type;
-for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-struct pci_region_entry *entry = busses[bus].r[type].list;
-while (entry) {
-pci_region_map_one_entry(busses, entry);
-struct pci_region_entry *next = entry-next;
-free(entry);
-entry = next;
-}
-}
+for (type = 0; type PCI_REGION_TYPE_COUNT; type++)
+pci_region_map_entries(busses, busses[bus].r[type]);
}
}
--
1.7.5.4
[Qemu-devel] [PATCH 09/12] pciinit: 64bit capability discovery for pci bridges
Add discovery if bridge region is 64bit is capable.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 26 +-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index f185cbd..0d66dbe 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -347,6 +347,28 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
*pis64 = is64;
}
+static int pci_bios_bridge_region_is64(struct pci_region *r,
+ struct pci_device *pci, int type)
+{
+if (type != PCI_REGION_TYPE_PREFMEM)
+return 0;
+u32 pmem = pci_config_readl(pci-bdf, PCI_PREF_MEMORY_BASE);
+if (!pmem) {
+pci_config_writel(pci-bdf, PCI_PREF_MEMORY_BASE, 0xfff0fff0);
+pmem = pci_config_readl(pci-bdf, PCI_PREF_MEMORY_BASE);
+pci_config_writel(pci-bdf, PCI_PREF_MEMORY_BASE, 0x0);
+}
+if ((pmem PCI_PREF_RANGE_TYPE_MASK) != PCI_PREF_RANGE_TYPE_64)
+ return 0;
+struct pci_region_entry *entry = r-list;
+while (entry) {
+if (!entry-is64)
+return 0;
+entry = entry-next;
+}
+return 1;
+}
+
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u64 size, u64 align, int type, int is64)
@@ -427,9 +449,11 @@ static int pci_bios_check_devices(struct pci_bus *busses)
if (s-r[type].align align)
align = s-r[type].align;
u64 size = ALIGN(s-r[type].sum, align);
+int is64 = pci_bios_bridge_region_is64(s-r[type],
+s-bus_dev, type);
// entry-bar is -1 if the entry represents a bridge region
struct pci_region_entry *entry = pci_region_create_entry(
-parent, s-bus_dev, -1, size, align, type, 0);
+parent, s-bus_dev, -1, size, align, type, is64);
if (!entry)
return -1;
dprintf(1, PCI: secondary bus %d size %08llx type %s\n,
--
1.7.5.4
[Qemu-devel] [PATCH 10/12] Calculate pci region stats on demand
Do not store pci region stats - instead calulate the
sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 57
+++--
1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 0d66dbe..a6cf98b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -40,8 +40,6 @@ struct pci_region_entry {
};
struct pci_region {
-/* pci region stats */
-u64 sum, align;
/* pci region assignments */
u64 base;
struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct
pci_region *r,
return 1;
}
+static u64 pci_region_align(struct pci_region *r)
+{
+if (!r-list)
+return 1;
+// The first entry in the sorted list has the largest alignment
+return r-list-align;
+}
+
+static u64 pci_region_sum(struct pci_region *r)
+{
+struct pci_region_entry *entry = r-list;
+u64 sum = 0;
+while (entry) {
+sum += entry-size;
+entry = entry-next;
+ }
+ return sum;
+}
+
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u64 size, u64 align, int type, int
is64)
@@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
}
entry-next = *pprev;
*pprev = entry;
-
-bus-r[type].sum += size;
-if (bus-r[type].align align)
-bus-r[type].align = align;
return entry;
}
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
u64 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-if (s-r[type].align align)
-align = s-r[type].align;
-u64 size = ALIGN(s-r[type].sum, align);
+if (pci_region_align(s-r[type]) align)
+ align = pci_region_align(s-r[type]);
+u64 size = ALIGN(pci_region_sum(s-r[type]), align);
int is64 = pci_bios_bridge_region_is64(s-r[type],
s-bus_dev, type);
// entry-bar is -1 if the entry represents a bridge region
@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
return 0;
}
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
-
// Setup region bases (given the regions' size and alignment)
static void pci_bios_init_root_regions(struct pci_bus *bus)
{
-u64 start = BUILD_PCIMEM_START;
-u64 end = BUILD_PCIMEM_END;
-
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
-int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-if (bus-r[reg1].align bus-r[reg2].align) {
+struct pci_region *r_end = bus-r[PCI_REGION_TYPE_PREFMEM];
+struct pci_region *r_start = bus-r[PCI_REGION_TYPE_MEM];
+
+if (pci_region_align(r_start) pci_region_align(r_end)) {
// Swap regions to improve alignment.
-reg1 = PCI_REGION_TYPE_MEM;
-reg2 = PCI_REGION_TYPE_PREFMEM;
+r_end = r_start;
+r_start = bus-r[PCI_REGION_TYPE_PREFMEM];
}
-bus-r[reg2].base = ROOT_BASE(end, bus-r[reg2].sum,
bus-r[reg2].align);
-bus-r[reg1].base = ROOT_BASE(bus-r[reg2].base, bus-r[reg1].sum
- , bus-r[reg1].align);
-if (bus-r[reg1].base start)
+r_end-base = ALIGN_DOWN((BUILD_PCIMEM_END -
pci_region_sum(r_end)),
+pci_region_align(r_end));
+r_start-base = ALIGN_DOWN((r_end-base - pci_region_sum(r_start)),
+pci_region_align(r_start));
+
+if ((r_start-base BUILD_PCIMEM_START) ||
+ (r_start-base BUILD_PCIMEM_END))
// Memory range requested is larger than available.
panic(PCI: out of address space\n);
}
--
1.7.5.4
[Qemu-devel] [PATCH 10/12] Calculate pci region stats on demand
Do not store pci region stats - instead calulate the
sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 57 +++--
1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 0d66dbe..a6cf98b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -40,8 +40,6 @@ struct pci_region_entry {
};
struct pci_region {
-/* pci region stats */
-u64 sum, align;
/* pci region assignments */
u64 base;
struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct pci_region
*r,
return 1;
}
+static u64 pci_region_align(struct pci_region *r)
+{
+if (!r-list)
+return 1;
+// The first entry in the sorted list has the largest alignment
+return r-list-align;
+}
+
+static u64 pci_region_sum(struct pci_region *r)
+{
+struct pci_region_entry *entry = r-list;
+u64 sum = 0;
+while (entry) {
+sum += entry-size;
+entry = entry-next;
+ }
+ return sum;
+}
+
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u64 size, u64 align, int type, int is64)
@@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
}
entry-next = *pprev;
*pprev = entry;
-
-bus-r[type].sum += size;
-if (bus-r[type].align align)
-bus-r[type].align = align;
return entry;
}
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus *busses)
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
u64 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-if (s-r[type].align align)
-align = s-r[type].align;
-u64 size = ALIGN(s-r[type].sum, align);
+if (pci_region_align(s-r[type]) align)
+ align = pci_region_align(s-r[type]);
+u64 size = ALIGN(pci_region_sum(s-r[type]), align);
int is64 = pci_bios_bridge_region_is64(s-r[type],
s-bus_dev, type);
// entry-bar is -1 if the entry represents a bridge region
@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus *busses)
return 0;
}
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
-
// Setup region bases (given the regions' size and alignment)
static void pci_bios_init_root_regions(struct pci_bus *bus)
{
-u64 start = BUILD_PCIMEM_START;
-u64 end = BUILD_PCIMEM_END;
-
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
-int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-if (bus-r[reg1].align bus-r[reg2].align) {
+struct pci_region *r_end = bus-r[PCI_REGION_TYPE_PREFMEM];
+struct pci_region *r_start = bus-r[PCI_REGION_TYPE_MEM];
+
+if (pci_region_align(r_start) pci_region_align(r_end)) {
// Swap regions to improve alignment.
-reg1 = PCI_REGION_TYPE_MEM;
-reg2 = PCI_REGION_TYPE_PREFMEM;
+r_end = r_start;
+r_start = bus-r[PCI_REGION_TYPE_PREFMEM];
}
-bus-r[reg2].base = ROOT_BASE(end, bus-r[reg2].sum, bus-r[reg2].align);
-bus-r[reg1].base = ROOT_BASE(bus-r[reg2].base, bus-r[reg1].sum
- , bus-r[reg1].align);
-if (bus-r[reg1].base start)
+r_end-base = ALIGN_DOWN((BUILD_PCIMEM_END - pci_region_sum(r_end)),
+pci_region_align(r_end));
+r_start-base = ALIGN_DOWN((r_end-base - pci_region_sum(r_start)),
+pci_region_align(r_start));
+
+if ((r_start-base BUILD_PCIMEM_START) ||
+ (r_start-base BUILD_PCIMEM_END))
// Memory range requested is larger than available.
panic(PCI: out of address space\n);
}
--
1.7.5.4
[Qemu-devel] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions
Migrate 64bit entries to 64bit pci regions if they do
not fit in 32bit range.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/config.h |2 ++
src/pciinit.c | 50 ++
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/config.h b/src/config.h
index b0187a4..bbacae7 100644
--- a/src/config.h
+++ b/src/config.h
@@ -47,6 +47,8 @@
#define BUILD_PCIMEM_START0xe000
#define BUILD_PCIMEM_END 0xfec0/* IOAPIC is mapped at */
+#define BUILD_PCIMEM64_START 0x80ULL
+#define BUILD_PCIMEM64_END0x100ULL
#define BUILD_IOAPIC_ADDR 0xfec0
#define BUILD_HPET_ADDRESS0xfed0
diff --git a/src/pciinit.c b/src/pciinit.c
index a6cf98b..3d7640b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -386,6 +386,31 @@ static u64 pci_region_sum(struct pci_region *r)
return sum;
}
+static void pci_region_migrate_64bit_entries(struct pci_region *from,
+ struct pci_region *to)
+{
+struct pci_region_entry **pprev = from-list;
+struct pci_region_entry **last = to-list;
+while(*pprev) {
+if ((*pprev)-is64) {
+struct pci_region_entry *entry;
+entry = *pprev;
+/* Delete the entry and move next */
+*pprev = (*pprev)-next;
+/* Add entry at tail to keep a sorted order */
+entry-next = NULL;
+if (*last) {
+ (*last)-next = entry;
+last = (*last)-next;
+}
+else
+ (*last) = entry;
+}
+else
+pprev = (*pprev)-next;
+}
+}
+
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u64 size, u64 align, int type, int is64)
@@ -478,7 +503,7 @@ static int pci_bios_check_devices(struct pci_bus *busses)
}
// Setup region bases (given the regions' size and alignment)
-static void pci_bios_init_root_regions(struct pci_bus *bus)
+static int pci_bios_init_root_regions(struct pci_bus *bus)
{
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
@@ -498,7 +523,8 @@ static void pci_bios_init_root_regions(struct pci_bus *bus)
if ((r_start-base BUILD_PCIMEM_START) ||
(r_start-base BUILD_PCIMEM_END))
// Memory range requested is larger than available.
-panic(PCI: out of address space\n);
+return -1;
+return 0;
}
@@ -561,6 +587,24 @@ static void pci_region_map_entries(struct pci_bus *busses,
struct pci_region *r)
static void pci_bios_map_devices(struct pci_bus *busses)
{
+if (pci_bios_init_root_regions(busses)) {
+struct pci_region r64_mem, r64_pref;
+r64_mem.list = NULL;
+r64_pref.list = NULL;
+pci_region_migrate_64bit_entries(busses[0].r[PCI_REGION_TYPE_MEM],
+ r64_mem);
+pci_region_migrate_64bit_entries(busses[0].r[PCI_REGION_TYPE_PREFMEM],
+ r64_pref);
+
+if (pci_bios_init_root_regions(busses))
+panic(PCI: out of address space\n);
+
+r64_mem.base = BUILD_PCIMEM64_START;
+r64_pref.base = ALIGN(r64_mem.base + pci_region_sum(r64_mem),
+ pci_region_align(r64_pref));
+pci_region_map_entries(busses, r64_mem);
+pci_region_map_entries(busses, r64_pref);
+}
// Map regions on each device.
int bus;
for (bus = 0; bus=MaxPCIBus; bus++) {
@@ -605,8 +649,6 @@ pci_setup(void)
if (pci_bios_check_devices(busses))
return;
-pci_bios_init_root_regions(busses[0]);
-
dprintf(1, === PCI new allocation pass #2 ===\n);
pci_bios_map_devices(busses);
--
1.7.5.4
[Qemu-devel] [PATCH 12/12] Fix 64bit PCI issues on Windows
This patch solves issues on Windows guests, when 64bit
BAR's are present. It is also helpful on Linux guests
when use_crs kernel boot option is set.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
src/acpi-dsdt.dsl |7 +
src/acpi-dsdt.hex | 64 +---
2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 4bdc268..4a18617 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -175,6 +175,13 @@ DefinitionBlock (
0x, // Address Translation Offset
0x1EC0, // Address Length
,, , AddressRangeMemory, TypeStatic)
+QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
Cacheable, ReadWrite,
+0x, // Address Space Granularity
+0x80,// Address Range Minimum
+0xFF,// Address Range Maximum
+0x, // Address Translation Offset
+0x80,// Address Length
+,, , AddressRangeMemory, TypeStatic)
})
}
}
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index a4af597..07f0e18 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
0x53,
0x44,
0x54,
-0x21,
+0x4f,
0x11,
0x0,
0x0,
0x1,
-0xe8,
+0xca,
0x42,
0x58,
0x50,
@@ -110,16 +110,16 @@ static unsigned char AmlCode[] = {
0x47,
0x42,
0x10,
-0x44,
-0x81,
+0x42,
+0x84,
0x5f,
0x53,
0x42,
0x5f,
0x5b,
0x82,
-0x4c,
-0x80,
+0x4a,
+0x83,
0x50,
0x43,
0x49,
@@ -2064,10 +2064,10 @@ static unsigned char AmlCode[] = {
0x52,
0x53,
0x11,
-0x42,
-0x7,
+0x40,
0xa,
-0x6e,
+0xa,
+0x9c,
0x88,
0xd,
0x0,
@@ -2176,6 +2176,52 @@ static unsigned char AmlCode[] = {
0x0,
0xc0,
0x1e,
+0x8a,
+0x2b,
+0x0,
+0x0,
+0xc,
+0x3,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
+0xff,
+0xff,
+0xff,
+0xff,
+0xff,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x0,
+0x80,
+0x0,
+0x0,
+0x0,
0x79,
0x0,
0x10,
--
1.7.5.4
[Qemu-devel] [PATCH 0/1] Add support for iSCSI thin-provisioning
List, Please find a patch that updates the iscsi block device to provide support for 1, LUNs bigger than 2TB by switching to READCAPACITY16 2, Thin-provisioning by implementing bdrv_aio_discard using SCSI UNMAP commands. The unmapping/discard of blocks was done by booting a guest and using sg_write_same --16 --unmap from within the guest. This was trapped in hw/scsi-disc.c and then appeared on the wire to the iscsi target as a proper SCSI UNMAP command. regards ronnie sahlberg
[Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs
Update the configure test for libiscsi support to detect version 1.3 or later.
Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP commands.
Update the iscsi block layer to use READCAPACITY16 to detect the size of the
LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB.
Update to implement bdrv_aio_discard() using the UNMAP command.
This allows us to use thin-provisioned LUNs from TGTD and other iSCSI targets
that support thin-provisioning.
Signed-off-by: Ronnie Sahlberg ronniesahlb...@gmail.com
---
block/iscsi.c | 86
configure |5 ++-
2 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index bd3ca11..eb49093 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -383,6 +383,65 @@ iscsi_aio_flush(BlockDriverState *bs,
return acb-common;
}
+static void
+iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
+ void *command_data, void *opaque)
+{
+IscsiAIOCB *acb = opaque;
+
+if (acb-canceled != 0) {
+qemu_aio_release(acb);
+scsi_free_scsi_task(acb-task);
+acb-task = NULL;
+return;
+}
+
+acb-status = 0;
+if (status 0) {
+error_report(Failed to unmap data on iSCSI lun. %s,
+ iscsi_get_error(iscsi));
+acb-status = -EIO;
+}
+
+iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+scsi_free_scsi_task(acb-task);
+acb-task = NULL;
+}
+
+static BlockDriverAIOCB *
+iscsi_aio_discard(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ BlockDriverCompletionFunc *cb, void *opaque)
+{
+IscsiLun *iscsilun = bs-opaque;
+struct iscsi_context *iscsi = iscsilun-iscsi;
+IscsiAIOCB *acb;
+struct unmap_list list[1];
+
+acb = qemu_aio_get(iscsi_aio_pool, bs, cb, opaque);
+
+acb-iscsilun = iscsilun;
+acb-canceled = 0;
+
+list[0].lba = sector_qemu2lun(sector_num, iscsilun);
+list[0].num = nb_sectors * BDRV_SECTOR_SIZE / iscsilun-block_size;
+
+acb-task = iscsi_unmap_task(iscsi, iscsilun-lun,
+ 0, 0, list[0], 1,
+ iscsi_unmap_cb,
+ acb);
+if (acb-task == NULL) {
+error_report(iSCSI: Failed to send unmap command. %s,
+ iscsi_get_error(iscsi));
+qemu_aio_release(acb);
+return NULL;
+}
+
+iscsi_set_events(iscsilun);
+
+return acb-common;
+}
+
static int64_t
iscsi_getlength(BlockDriverState *bs)
{
@@ -396,11 +455,11 @@ iscsi_getlength(BlockDriverState *bs)
}
static void
-iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int status,
+iscsi_readcapacity16_cb(struct iscsi_context *iscsi, int status,
void *command_data, void *opaque)
{
struct IscsiTask *itask = opaque;
-struct scsi_readcapacity10 *rc10;
+struct scsi_readcapacity16 *rc16;
struct scsi_task *task = command_data;
if (status != 0) {
@@ -412,26 +471,25 @@ iscsi_readcapacity10_cb(struct iscsi_context *iscsi, int
status,
return;
}
-rc10 = scsi_datain_unmarshall(task);
-if (rc10 == NULL) {
-error_report(iSCSI: Failed to unmarshall readcapacity10 data.);
+rc16 = scsi_datain_unmarshall(task);
+if (rc16 == NULL) {
+error_report(iSCSI: Failed to unmarshall readcapacity16 data.);
itask-status = 1;
itask-complete = 1;
scsi_free_scsi_task(task);
return;
}
-itask-iscsilun-block_size = rc10-block_size;
-itask-iscsilun-num_blocks = rc10-lba;
-itask-bs-total_sectors = (uint64_t)rc10-lba *
- rc10-block_size / BDRV_SECTOR_SIZE ;
+itask-iscsilun-block_size = rc16-block_length;
+itask-iscsilun-num_blocks = rc16-returned_lba;
+itask-bs-total_sectors= rc16-returned_lba *
+ rc16-block_length / BDRV_SECTOR_SIZE ;
itask-status = 0;
itask-complete = 1;
scsi_free_scsi_task(task);
}
-
static void
iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
void *opaque)
@@ -445,10 +503,10 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status,
void *command_data,
return;
}
-task = iscsi_readcapacity10_task(iscsi, itask-iscsilun-lun, 0, 0,
- iscsi_readcapacity10_cb, opaque);
+task = iscsi_readcapacity16_task(iscsi, itask-iscsilun-lun,
+ iscsi_readcapacity16_cb, opaque);
if (task == NULL) {
-error_report(iSCSI: failed to send readcapacity command.);
+error_report(iSCSI: failed to send readcapacity16 command.);
itask-status = 1;
itask-complete = 1;
return;
@@ -700,6 +758,8 @@ static BlockDriver bdrv_iscsi = {
.bdrv_aio_readv =
Re: [Qemu-devel] [SeaBIOS] [PATCH 10/12] Calculate pci region stats on demand
A flowed text. Please apply another [10/12] in this series.
On Tue, 2012-04-24 at 18:23 +1200, Alexey Korolev wrote:
Do not store pci region stats - instead calulate the
sum and alignment on demand.
Signed-off-by: Alexey Korolev alexey.koro...@endace.com
---
src/pciinit.c | 57
+++--
1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 0d66dbe..a6cf98b 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -40,8 +40,6 @@ struct pci_region_entry {
};
struct pci_region {
-/* pci region stats */
-u64 sum, align;
/* pci region assignments */
u64 base;
struct pci_region_entry *list;
@@ -369,6 +367,25 @@ static int pci_bios_bridge_region_is64(struct
pci_region *r,
return 1;
}
+static u64 pci_region_align(struct pci_region *r)
+{
+if (!r-list)
+return 1;
+// The first entry in the sorted list has the largest alignment
+return r-list-align;
+}
+
+static u64 pci_region_sum(struct pci_region *r)
+{
+struct pci_region_entry *entry = r-list;
+u64 sum = 0;
+while (entry) {
+sum += entry-size;
+entry = entry-next;
+ }
+ return sum;
+}
+
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
int bar, u64 size, u64 align, int type, int
is64)
@@ -394,10 +411,6 @@ pci_region_create_entry(struct pci_bus *bus, struct
pci_device *dev,
}
entry-next = *pprev;
*pprev = entry;
-
-bus-r[type].sum += size;
-if (bus-r[type].align align)
-bus-r[type].align = align;
return entry;
}
@@ -446,9 +459,9 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
u64 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-if (s-r[type].align align)
-align = s-r[type].align;
-u64 size = ALIGN(s-r[type].sum, align);
+if (pci_region_align(s-r[type]) align)
+ align = pci_region_align(s-r[type]);
+u64 size = ALIGN(pci_region_sum(s-r[type]), align);
int is64 = pci_bios_bridge_region_is64(s-r[type],
s-bus_dev, type);
// entry-bar is -1 if the entry represents a bridge region
@@ -464,26 +477,26 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
return 0;
}
-#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
-
// Setup region bases (given the regions' size and alignment)
static void pci_bios_init_root_regions(struct pci_bus *bus)
{
-u64 start = BUILD_PCIMEM_START;
-u64 end = BUILD_PCIMEM_END;
-
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
-int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
-if (bus-r[reg1].align bus-r[reg2].align) {
+struct pci_region *r_end = bus-r[PCI_REGION_TYPE_PREFMEM];
+struct pci_region *r_start = bus-r[PCI_REGION_TYPE_MEM];
+
+if (pci_region_align(r_start) pci_region_align(r_end)) {
// Swap regions to improve alignment.
-reg1 = PCI_REGION_TYPE_MEM;
-reg2 = PCI_REGION_TYPE_PREFMEM;
+r_end = r_start;
+r_start = bus-r[PCI_REGION_TYPE_PREFMEM];
}
-bus-r[reg2].base = ROOT_BASE(end, bus-r[reg2].sum,
bus-r[reg2].align);
-bus-r[reg1].base = ROOT_BASE(bus-r[reg2].base, bus-r[reg1].sum
- , bus-r[reg1].align);
-if (bus-r[reg1].base start)
+r_end-base = ALIGN_DOWN((BUILD_PCIMEM_END -
pci_region_sum(r_end)),
+pci_region_align(r_end));
+r_start-base = ALIGN_DOWN((r_end-base - pci_region_sum(r_start)),
+pci_region_align(r_start));
+
+if ((r_start-base BUILD_PCIMEM_START) ||
+ (r_start-base BUILD_PCIMEM_END))
// Memory range requested is larger than available.
panic(PCI: out of address space\n);
}
Re: [Qemu-devel] [PATCH] scsi refcounting fix?
Il 24/04/2012 07:02, David Gibson ha scritto:
So the patch below fixes my assertion failure, but again, I can't say
I understand this well enough to be confident - it might be leaking
scsi reqs instead. But if this isn't the right fix, I hope one of you
can help me find where the real refcounting bug is.
Thanks for the report!
The fix seems correct. However, since refcounting is tricky, I prefer
to follow existing patterns and make scsi_do_read look like a combination
of scsi_*_complete + scsi_*_data. The following does both a ref
(like in scsi_read_data) and and an unref (like in scsi_flush_complete):
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a914611..49f5860 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -297,6 +297,13 @@ static void scsi_do_read(void *opaque, int ret)
}
}
+if (r-req.io_canceled) {
+return;
+}
+
+/* The request is used as the AIO opaque value, so add a ref. */
+scsi_req_ref(r-req);
+
if (r-req.sg) {
dma_acct_start(s-qdev.conf.bs, r-acct, r-req.sg, BDRV_ACCT_READ);
r-req.resid -= r-req.sg-size;
Can you confirm that this works for you?
Paolo
Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs
Il 24/04/2012 08:29, Ronnie Sahlberg ha scritto: Update the configure test for libiscsi support to detect version 1.3 or later. Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP commands. Update the iscsi block layer to use READCAPACITY16 to detect the size of the LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB. Update to implement bdrv_aio_discard() using the UNMAP command. This allows us to use thin-provisioned LUNs from TGTD and other iSCSI targets that support thin-provisioning. Looks good. Kevin, do you want me to take libiscsi patches via the SCSI tree? As an aside, I am not really sure of the utility of adding these utility functions directly in libiscsi, rather than making it a pure transport library. block/iscsi.c is going to grow as you add more functionality (e.g. WRITE SAME commands), and libiscsi will have to be updated each time in lockstep. I can see the value of basic read/write/flush and readcap10/16, but with unmap it's starting to be a bit more specific. Are there other clients of libiscsi that use these functions? Should they be placed into block/iscsi.c or a new block/iscsi-cdb.c instead? Paolo
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Il 23/04/2012 18:06, Pavel Hrdina ha scritto:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the
drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no
media in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults
and both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina phrd...@redhat.com
Signed-off-by: Michal Novotny minov...@redhat.com
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs, nb_heads, max_track,
last_sect, drv-drive, drive, rate);
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags = ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
if (floppy) {
fdc_get_bs(fd, floppy);
for (i = 0; i 2; i++) {
-if (fd[i] bdrv_is_inserted(fd[i])) {
+/* If there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i], nb_heads, max_track,
last_sect, FDRIVE_DRV_NONE,
fd_type[i], rate);
Strictly speaking this final hunk should not be necessary: the fd_type
is by default FDRIVE_DRV_NONE and it is correct if there is no medium in
the drive. However, it does not hurt.
The rest of the patch looks good. It's strictly a bugfix and doesn't
change the hardware exposed to the guest (only the media), so
I think this does not require a compatibility property.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Paolo
Re: [Qemu-devel] [SeaBIOS] [PATCH 05/12] pciinit: Track region alignment explicitly.
On 04/24/12 08:17, Alexey Korolev wrote:
Don't round up bridge regions to the next highest size - instead track
alignment explicitly. This should improve the memory layout for
bridge regions.
This one got mangled too:
Applying: pciinit: Track region alignment explicitly.
fatal: corrupt patch at line 40
Patch failed at 0005 pciinit: Track region alignment explicitly.
Do you have a git tree I can pull from for testing?
thanks,
Gerd
Also, unused bridge regions will no longer be allocated any space.
Signed-off-by: Kevin O'Connor ke...@koconnor.net
---
src/pciinit.c | 41 ++---
1 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/pciinit.c b/src/pciinit.c
index 1b31177..2bd4426 100644
--- a/src/pciinit.c
+++ b/src/pciinit.c
@@ -33,6 +33,7 @@ struct pci_region_entry {
struct pci_device *dev;
int bar;
u32 size;
+u32 align;
int is64;
enum pci_region_type type;
struct pci_region_entry *next;
@@ -41,7 +42,7 @@ struct pci_region_entry {
struct pci_bus {
struct {
/* pci region stats */
-u32 sum, max;
+u32 sum, align;
/* pci region assignments */
u32 base;
struct pci_region_entry *list;
@@ -307,12 +308,6 @@ pci_bios_init_bus(void)
* Bus sizing
/
-static u32 pci_size_roundup(u32 size)
-{
-int index = __fls(size-1)+1;
-return 0x1 index;
-}
-
static void
pci_bios_get_bar(struct pci_device *pci, int bar, u32 *val, u32 *size)
{
@@ -338,7 +333,7 @@ pci_bios_get_bar(struct pci_device *pci, int bar,
u32 *val, u32 *size)
static struct pci_region_entry *
pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
-int bar, u32 size, int type, int is64)
+int bar, u32 size, u32 align, int type, int
is64)
{
struct pci_region_entry *entry = malloc_tmp(sizeof(*entry));
if (!entry) {
@@ -349,21 +344,22 @@ pci_region_create_entry(struct pci_bus *bus,
struct pci_device *dev,
entry-dev = dev;
entry-bar = bar;
entry-size = size;
+entry-align = align;
entry-is64 = is64;
entry-type = type;
// Insert into list in sorted order.
struct pci_region_entry **pprev;
for (pprev = bus-r[type].list; *pprev; pprev = (*pprev)-next) {
struct pci_region_entry *pos = *pprev;
-if (pos-size size)
+if (pos-align align || (pos-align == align pos-size
size))
break;
}
entry-next = *pprev;
*pprev = entry;
bus-r[type].sum += size;
-if (bus-r[type].max size)
-bus-r[type].max = size;
+if (bus-r[type].align align)
+bus-r[type].align = align;
return entry;
}
@@ -393,7 +389,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
(val PCI_BASE_ADDRESS_MEM_TYPE_MASK)
== PCI_BASE_ADDRESS_MEM_TYPE_64);
struct pci_region_entry *entry = pci_region_create_entry(
-bus, pci, i, size, type, is64);
+bus, pci, i, size, size, type, is64);
if (!entry)
return -1;
@@ -411,15 +407,14 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
struct pci_bus *parent =
busses[pci_bdf_to_bus(s-bus_dev-bdf)];
int type;
for (type = 0; type PCI_REGION_TYPE_COUNT; type++) {
-u32 limit = (type == PCI_REGION_TYPE_IO) ?
+u32 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
-u32 size = s-r[type].sum;
-if (size limit)
-size = limit;
-size = pci_size_roundup(size);
+if (s-r[type].align align)
+align = s-r[type].align;
+u32 size = ALIGN(s-r[type].sum, align);
// entry-bar is -1 if the entry represents a bridge region
struct pci_region_entry *entry = pci_region_create_entry(
-parent, s-bus_dev, -1, size, type, 0);
+parent, s-bus_dev, -1, size, align, type, 0);
if (!entry)
return -1;
dprintf(1, PCI: secondary bus %d size %x type %s\n,
@@ -430,7 +425,7 @@ static int pci_bios_check_devices(struct pci_bus
*busses)
return 0;
}
-#define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
+#define ROOT_BASE(top, sum, align) ALIGN_DOWN((top)-(sum),(align) ?: 1)
// Setup region bases (given the regions' size and alignment)
static int pci_bios_init_root_regions(struct pci_bus *bus, u32 start,
u32 end)
@@ -438,14 +433,14 @@ static int pci_bios_init_root_regions(struct
pci_bus *bus, u32 start, u32 end)
bus-r[PCI_REGION_TYPE_IO].base = 0xc000;
int reg1 = PCI_REGION_TYPE_PREFMEM, reg2
Re: [Qemu-devel] [PATCH v6 2/5] Implement i.MX31 Clock Control Module
Il 23/04/2012 22:54, Peter Chubb ha scritto: Peter What is this calculation supposed to do? It doesn't convert a Peter 10-bit signed twos-complement number into an int32_t, unless Peter I'm confused... Also, it's a rather opaque way to write mfn = Peter 0x200;. I'll use a different way to calculate. Maybe: mfn = (32-10); mfn = (32-10); The magic that you wanted is mfn |= -(mfn 0x200); :) but I do prefer the shifts too. Paolo
Re: [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation
On Tue, Apr 24, 2012 at 01:26:43AM +0900, MORITA Kazutaka wrote: SD_FLAG_CMD_CACHE is ignored in the older version of Sheepdog, so, even if we specify cache=writeback or cache=none, the data is written with O_DSYNC always and cannot be cached in the server's page cache or volatile disk cache either. I think it is safe. It seems that there is another version problem with this patch; bdrv_co_flush_to_disk() results in error with the older Sheepdog which doesn't support SD_OP_FLUSH_VDI. The simple fix is to increment SD_PROTO_VER and prevent the newer qemu from connecting to the older Sheepdog cluster, I think. Incrementin the version seems like a must. But qemu can still connect to older protocol versions fine if connecting to the current one fails, t just must not send SD_OP_FLUSH_VDI (and preferably not set SD_FLAG_CMD_CACHE) in that case.
Re: [Qemu-devel] [PATCH 00/16] QEMU vhost-scsi support
Il 24/04/2012 06:21, ronnie sahlberg ha scritto: Hi Stefan, A little bit off-topic but When you design the proper place and API to plug virt-scsi into an external SCSI parser outside of qemu like the target in the kernel ... It would be very nice if one could also plug virt-scsi into libiscsi and pass the CDBs straight to the remote iSCSI target too. Keep some thoughts on virt-scsi + libiscsi integration. Yes, that makes a lot of sense. It's a bit harder than scsi-generic but we do want to get there. Paolo
[Qemu-devel] [PATCH] docs: fix one issue in qcow2 specs
From: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- docs/specs/qcow2.txt |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt index b6adcad..ae68a6e 100644 --- a/docs/specs/qcow2.txt +++ b/docs/specs/qcow2.txt @@ -185,7 +185,7 @@ L2 table entry (for normal clusters): in L2 tables that are reachable from the the active L1 table. -L2 table entry (for compressed clusters; x = 62 - (cluster_size - 8)): +L2 table entry (for compressed clusters; x = 62 - (cluster_bits - 8)): Bit 0 - x:Host cluster offset. This is usually _not_ aligned to a cluster boundary! -- 1.7.6
Re: [Qemu-devel] [PATCH v6 2/5] Implement i.MX31 Clock Control Module
Paolo == Paolo Bonzini pbonz...@redhat.com writes: Paolo Il 23/04/2012 22:54, Peter Chubb ha scritto: Peter What is this calculation supposed to do? It doesn't convert a Peter 10-bit signed twos-complement number into an int32_t, unless Peter I'm confused... Also, it's a rather opaque way to write mfn = Peter 0x200;. I'll use a different way to calculate. Maybe: mfn = (32-10); mfn = (32-10); Paolo The magic that you wanted is Paolomfn |= -(mfn 0x200); Yes. I'd actually been thinking of mfn -= 2 * (mfn 0x200); but forgot the 2*. But the shifts should be faster, and that's wjhat I'm testing at the moment. Peter C -- Dr Peter Chubb peter.chubb AT nicta.com.au http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
[Qemu-devel] Fwd: buildbot failure in qemu on rhel5-default
Original Message Subject: buildbot failure in qemu on rhel5-default Date: Mon, 23 Apr 2012 23:31:44 +0200 From: build...@spunk.home.kraxel.org To: kra...@gmail.com The Buildbot has detected a failed build on builder rhel5-default while building qemu. Full details are available at: http://spunk.home.kraxel.org/bb/builders/rhel5-default/builds/249 Buildbot URL: http://spunk.home.kraxel.org/bb/ Buildslave for this Build: rhel5 Build Reason: scheduler Build Source Stamp: [branch master] 092dfc7714ad7983aeb0cada5d5983e9fde8d84c Blamelist: Amos Kong ak...@redhat.com,Andreas Färber afaer...@suse.de,Anthony Liguori aligu...@us.ibm.com,Blue Swirl blauwir...@gmail.com,David Gibson da...@gibson.dropbear.id.au,Dong Xu Wang wdon...@linux.vnet.ibm.com,Eduardo Elias Ferreira ed...@linux.vnet.ibm.com,Eric Bénard e...@eukrea.com,Kevin Wolf kw...@redhat.com,Liu Yuan tailai...@taobao.com,Lluís Vilanova vilan...@ac.upc.edu,Michael Roth mdr...@linux.vnet.ibm.com,NODA, Kai noda...@gmail.com,Paolo Bonzini pbonz...@redhat.com,Ronnie Sahlberg ronniesahlb...@gmail.com,Stefan Hajnoczi stefa...@linux.vnet.ibm.com,Stefan Weil s...@weilnetz.de,Stefano Stabellini stefano.stabell...@eu.citrix.com BUILD FAILED: failed configure sincerely, -The Buildbot == log tail == Error: invalid trace backend Please choose a supported trace backend. == full log == [ Note: IPv6 connectivity needed to access this ] http://spunk.home.kraxel.org/bb/builders/rhel5-default/builds/249/steps/configure/logs/stdio
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 08:55 AM, Paolo Bonzini wrote:
Il 23/04/2012 18:06, Pavel Hrdina ha scritto:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdinaphrd...@redhat.com
Signed-off-by: Michal Novotnyminov...@redhat.com
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs,nb_heads,max_track,
last_sect, drv-drive,drive,rate);
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
if (floppy) {
fdc_get_bs(fd, floppy);
for (i = 0; i 2; i++) {
-if (fd[i] bdrv_is_inserted(fd[i])) {
+/* If there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i],nb_heads,max_track,
last_sect, FDRIVE_DRV_NONE,
fd_type[i],rate);
Strictly speaking this final hunk should not be necessary: the fd_type
is by default FDRIVE_DRV_NONE and it is correct if there is no medium in
the drive. However, it does not hurt.
The rest of the patch looks good. It's strictly a bugfix and doesn't
change the hardware exposed to the guest (only the media), so
I think this does not require a compatibility property.
Reviewed-by: Paolo Bonzinipbonz...@redhat.com
Paolo
Hi,
there is bug that you cannot see any floppy drive in guest if you use
defaults or set up a floppy drive without media. On bare-metal you can
see floppy drive even without media.
For qemu you can check that there is floppy drive present by using qemu
monitor and info block. After you inserted media to the floppy drive
on running guest, you still cannot see any floppy drive in guest.
This patch will set floppy drive to default values if you start guest
with floppy drive without media and after you insert proper media to
floppy drive you can access it from guest.
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the
drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no
media in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults
and both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina phrd...@redhat.com
Signed-off-by: Michal Novotny minov...@redhat.com
---
Please see http://wiki.qemu.org/Contribute/SubmitAPatch for hints on
writing a commit message for a patch: Subject should have a prefix, such
as fdc: Fix emulation for no media and the body should not contain
personal letter-style contents such as Hi, this is the patch. Any such
personal comments can go under the --- line where they are not committed
to git history.
Cc'ing floppy author and block maintainer.
Andreas
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs, nb_heads, max_track,
last_sect, drv-drive, drive, rate);
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags = ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
if (floppy) {
fdc_get_bs(fd, floppy);
for (i = 0; i 2; i++) {
-if (fd[i] bdrv_is_inserted(fd[i])) {
+/* If there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i], nb_heads, max_track,
last_sect, FDRIVE_DRV_NONE,
fd_type[i], rate);
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v6 0/5] FreeSCALE i.MX31 support
Am 23.04.2012 01:31, schrieb Peter Chubb: Hi all, Most of the files are unchanged since last time. Indeed... On v5 I had asked you to shorten the subjects to conform to our commit message scheme and to make patches better readable. There were even suggestions. Also I implicitly suggested to use one spelling of Freescale consistently throughout your patches. Did you forget? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs
On Tue, Apr 24, 2012 at 4:46 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 24/04/2012 08:29, Ronnie Sahlberg ha scritto: Update the configure test for libiscsi support to detect version 1.3 or later. Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP commands. Update the iscsi block layer to use READCAPACITY16 to detect the size of the LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB. Update to implement bdrv_aio_discard() using the UNMAP command. This allows us to use thin-provisioned LUNs from TGTD and other iSCSI targets that support thin-provisioning. Looks good. Kevin, do you want me to take libiscsi patches via the SCSI tree? As an aside, I am not really sure of the utility of adding these utility functions directly in libiscsi, rather than making it a pure transport library. block/iscsi.c is going to grow as you add more functionality (e.g. WRITE SAME commands), and libiscsi will have to be updated each time in lockstep. I can see the value of basic read/write/flush and readcap10/16, but with unmap it's starting to be a bit more specific. Are there other clients of libiscsi that use these functions? Should they be placed into block/iscsi.c or a new block/iscsi-cdb.c instead? I see your point. I like to add scsi commands as I use them to libiscsi, since then I dont have to re-write the marshalling/unmarshalling code everytime in my small test and utility programs. For example when i want to write some one-off small tools to test something. But, yes. There is no real need to use them directly from qemu. So ignore this patch for now. I will redo UNMAP in the patch to instead use the generic scsi function inside libiscsi. That will serve the purpose to verify that the public API in libiscsi is sufficient for accessing the generic transport and secondly that will show a useful example on how to send CDB+data to and to receive data back from the generic function. This generic API would be what a future virt-scsi-libiscsi integration would use anyway. regards ronnie sahlberg
Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs
Am 24.04.2012 08:46, schrieb Paolo Bonzini: Il 24/04/2012 08:29, Ronnie Sahlberg ha scritto: Update the configure test for libiscsi support to detect version 1.3 or later. Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP commands. Update the iscsi block layer to use READCAPACITY16 to detect the size of the LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB. Update to implement bdrv_aio_discard() using the UNMAP command. This allows us to use thin-provisioned LUNs from TGTD and other iSCSI targets that support thin-provisioning. Looks good. Kevin, do you want me to take libiscsi patches via the SCSI tree? Sure, if you like, go ahead. Feel free to update MAINTAINERS as well. As an aside, I am not really sure of the utility of adding these utility functions directly in libiscsi, rather than making it a pure transport library. block/iscsi.c is going to grow as you add more functionality (e.g. WRITE SAME commands), and libiscsi will have to be updated each time in lockstep. I can see the value of basic read/write/flush and readcap10/16, but with unmap it's starting to be a bit more specific. Are there other clients of libiscsi that use these functions? Should they be placed into block/iscsi.c or a new block/iscsi-cdb.c instead? I think I agree. For the more obscure commands, the qemu driver should probably build the CDB on its own and use a generic function. Kevin
Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug
On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote: Hi, On Sun, Apr 22, 2012 at 05:20:59PM +0300, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:13:27PM +0300, Avi Kivity wrote: On 04/22/2012 05:09 PM, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:06:43PM +0300, Avi Kivity wrote: On 04/22/2012 04:56 PM, Gleb Natapov wrote: start. We will need it for migration anyway. hotplug-able memory slots i.e. initial system memory is not modeled with memslots. The concept could be generalized to include all memory though, or it could more closely follow kvm-memory slots. OK, I hope final version will allow for memory 4G to be hot-pluggable. Why is that important? Because my feeling is that people that want to use this kind of feature what to start using it with VMs smaller than 4G. Of course not all memory have to be hot unpluggable. Making first 1M or event first 128M not unpluggable make perfect sense. Can't you achieve this with -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false? From this: (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so hotplugged memory should start from max(4G, above_4g_mem_size). I understand that hotpluggable memory can start from above 4G only. With the config above we will have memory hole from 1G to PCI memory hole. May be not a big problem, but I do not see technical reason for the constrain. The 440fx spec mentions: The address range from the top of main DRAM to 4 Gbytes (top of physical memory space supported by the 440FX PCIset) is normally mapped to PCI. The PMC forwards all accesses within this address range to PCI. What we probably want is that the initial memory map creation takes into account all dimms specified (both populated/unpopulated) Yes. So -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false would create a system map with top of memory and start of PCI-hole at 2G. What -m 1G means on this command line? Isn't it redundant? May be we should make -m create non unplaggable, populated slot starting at address 0. Ten you config above will specify 3G memory with 2G populated (first of which is not removable) and 1G unpopulated. PCI hole starts above 3G. This may require some shifting of physical address offsets around 3.5GB-4GB - is this the minimum PCI hole allowed? Currently it is 1G in QEMU code. E.g. if we specify 4x1GB DIMMs (onlt the first initially populated) -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false -device dimm,size=1G,populated=false -device dimm,size=1G,populated=false we create the following memory map: dimm0: [0,1G) dimm1: [1G, 2G) dimm2: [2G, 3G) dimm3: [4G, 5G) or dimm3 is split into [3G, 3.5G) and [4G, 4.5G) does either of these options sound reasonable? We shouldn't split dimms IMO. Just unnecessary complication. Better make bigger PCI hole. -- Gleb.
Re: [Qemu-devel] [PATCH] docs: fix one issue in qcow2 specs
Am 24.04.2012 09:11, schrieb zwu.ker...@gmail.com: From: Zhi Yong Wu wu...@linux.vnet.ibm.com Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- docs/specs/qcow2.txt |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt index b6adcad..ae68a6e 100644 --- a/docs/specs/qcow2.txt +++ b/docs/specs/qcow2.txt @@ -185,7 +185,7 @@ L2 table entry (for normal clusters): in L2 tables that are reachable from the the active L1 table. -L2 table entry (for compressed clusters; x = 62 - (cluster_size - 8)): +L2 table entry (for compressed clusters; x = 62 - (cluster_bits - 8)): Bit 0 - x:Host cluster offset. This is usually _not_ aligned to a cluster boundary! Good catch! Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs
Il 24/04/2012 10:02, ronnie sahlberg ha scritto: So ignore this patch for now. I will redo UNMAP in the patch to instead use the generic scsi function inside libiscsi. That will serve the purpose to verify that the public API in libiscsi is sufficient for accessing the generic transport and secondly that will show a useful example on how to send CDB+data to and to receive data back from the generic function. This generic API would be what a future virt-scsi-libiscsi integration would use anyway. I will be on holiday from tomorrow to May 1, and I won't be able to send a pull request today. As I want to minimize the time I spend looking at qemu-devel, :) I'll take this patch as is, since the new libiscsi version is needed anyway for READ CAPACITY (16). You could write a follow-up patch to teach iscsi.c about WRITE SAME(10) and WRITE SAME(16) with the unmap bit set, and use generic CDB+data functions for those. Paolo
Re: [Qemu-devel] [PATCH] scsi refcounting fix?
On Tue, Apr 24, 2012 at 08:37:22AM +0200, Paolo Bonzini wrote:
Il 24/04/2012 07:02, David Gibson ha scritto:
So the patch below fixes my assertion failure, but again, I can't say
I understand this well enough to be confident - it might be leaking
scsi reqs instead. But if this isn't the right fix, I hope one of you
can help me find where the real refcounting bug is.
Thanks for the report!
The fix seems correct. However, since refcounting is tricky, I prefer
to follow existing patterns and make scsi_do_read look like a combination
of scsi_*_complete + scsi_*_data. The following does both a ref
(like in scsi_read_data) and and an unref (like in scsi_flush_complete):
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a914611..49f5860 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -297,6 +297,13 @@ static void scsi_do_read(void *opaque, int ret)
}
}
+if (r-req.io_canceled) {
+return;
+}
+
+/* The request is used as the AIO opaque value, so add a ref. */
+scsi_req_ref(r-req);
+
if (r-req.sg) {
dma_acct_start(s-qdev.conf.bs, r-acct, r-req.sg, BDRV_ACCT_READ);
r-req.resid -= r-req.sg-size;
Can you confirm that this works for you?
This seems to work for me, thanks.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCHv2 0/3] virtio: fix memory access races
On Mon, Apr 23, 2012 at 2:19 PM, Michael S. Tsirkin m...@redhat.com wrote: This is a follow-up to my previous patch: it turns out that a single mb() isn't sufficient as network loss could still be triggered under stress. Patch 1 is repost of v1. The following two patches fix more races found by code inspection and comparison with vhost in kernel. After applying these patches, no more network loss was observed. Michael S. Tsirkin (3): virtio: add missing mb() on notification virtio: add missing mb() on enable notification virtio: order index/descriptor reads hw/virtio.c | 11 +++ qemu-barrier.h | 28 +--- 2 files changed, 36 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug
On Mon, Apr 23, 2012 at 04:31:04PM +0300, Avi Kivity wrote: On 04/22/2012 05:20 PM, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:13:27PM +0300, Avi Kivity wrote: On 04/22/2012 05:09 PM, Gleb Natapov wrote: On Sun, Apr 22, 2012 at 05:06:43PM +0300, Avi Kivity wrote: On 04/22/2012 04:56 PM, Gleb Natapov wrote: start. We will need it for migration anyway. hotplug-able memory slots i.e. initial system memory is not modeled with memslots. The concept could be generalized to include all memory though, or it could more closely follow kvm-memory slots. OK, I hope final version will allow for memory 4G to be hot-pluggable. Why is that important? Because my feeling is that people that want to use this kind of feature what to start using it with VMs smaller than 4G. Of course not all memory have to be hot unpluggable. Making first 1M or event first 128M not unpluggable make perfect sense. Can't you achieve this with -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false? From this: (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so hotplugged memory should start from max(4G, above_4g_mem_size). I understand that hotpluggable memory can start from above 4G only. With the config above we will have memory hole from 1G to PCI memory hole. May be not a big problem, but I do not see technical reason for the constrain. (I don't think hotplugging below 512MB is needed, but I don't have any real data on this). 512MB looks like a reasonable limitation too, but again if there is not technical reason for having the limitation why have it? I was thinking about not having tons of 128MB slots, so we don't have a configuration that is far from reality. But maybe this thinking is too conservative. I think it is good interface to make memory that is specified with -m to be one big unpluggable slot, but slots defined with -device should start just above what -m specifies (after proper alignment). Memory hot-plug granularity is controlled by slot's size parameter. -- Gleb.
Re: [Qemu-devel] [PATCH v6 0/5] FreeSCALE i.MX31 support
Andreas == Andreas Färber afaer...@suse.de writes: Andreas Am 23.04.2012 01:31, schrieb Peter Chubb: Hi all, Most of the files are unchanged since last time. Andreas Indeed... On v5 I had asked you to shorten the subjects to Andreas conform to our commit message scheme and to make patches Andreas better readable. There were even suggestions. Also I Andreas implicitly suggested to use one spelling of Freescale Andreas consistently throughout your patches. Sorry, yes I missed that comment. V7 will come soon. -- Dr Peter Chubb peter.chubb AT nicta.com.au http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA
Re: [Qemu-devel] [PATCH] block/qcow2: Add missing GCC_FMT_ATTR to function report_unsupported()
Am 23.04.2012 22:54, schrieb Stefan Weil:
Cc: Kevin Wolf kw...@redhat.com
Signed-off-by: Stefan Weil s...@weilnetz.de
---
block/qcow2.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ad46c03..d03e31c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -182,7 +182,7 @@ static void cleanup_unknown_header_ext(BlockDriverState
*bs)
}
}
-static void report_unsupported(BlockDriverState *bs, const char *fmt, ...)
+static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
const char *fmt, ...)
{
char msg[64];
va_list ap;
Thanks, fixed to have not more than 80 characters per line and applied
to the block branch.
Kevin
Re: [Qemu-devel] [RFC PATCH 3/9][SeaBIOS] acpi: generate hotplug memory devices.
Hi,
On Mon, Apr 23, 2012 at 07:37:51PM -0400, Kevin O'Connor wrote:
On Thu, Apr 19, 2012 at 04:08:41PM +0200, Vasilis Liaskovitis wrote:
The memory device generation is guided by qemu paravirt info. Seabios
first uses the info to setup SRAT entries for the hotplug-able memory
slots.
Afterwards, build_memssdt uses the created SRAT entries to generate
appropriate memory device objects. One memory device (and corresponding
SRAT
entry) is generated for each hotplug-able qemu memslot. Currently no SSDT
memory device is created for initial system memory (the method can be
generalized to all memory though).
Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
---
src/acpi.c | 151
++--
1 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/src/acpi.c b/src/acpi.c
index 30888b9..5580099 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -484,6 +484,131 @@ build_ssdt(void)
return ssdt;
}
+static unsigned char ssdt_mem[] = {
+0x5b,0x82,0x47,0x07,0x4d,0x50,0x41,0x41,
This patch looks like it uses the SSDT generation mechanism that was
present in SeaBIOS v1.6.3. Since then, however, the runtime AML code
generation has been improved to be more dynamic. Any runtime
generated AML code should be updated to use the newer mechanisms.
thanks, I will look into the new mechanism and rewrite.
- Vasilis
Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug
On Tue, Apr 24, 2012 at 10:24:51AM +0200, Vasilis Liaskovitis wrote: Hi, On Tue, Apr 24, 2012 at 10:52:24AM +0300, Gleb Natapov wrote: On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote: The 440fx spec mentions: The address range from the top of main DRAM to 4 Gbytes (top of physical memory space supported by the 440FX PCIset) is normally mapped to PCI. The PMC forwards all accesses within this address range to PCI. What we probably want is that the initial memory map creation takes into account all dimms specified (both populated/unpopulated) Yes. So -m 1G, -device dimm,size=1G,populated=true -device dimm,size=1G,populated=false would create a system map with top of memory and start of PCI-hole at 2G. What -m 1G means on this command line? Isn't it redundant? yes, this was redundant with the original concept. May be we should make -m create non unplaggable, populated slot starting at address 0. Ten you config above will specify 3G memory with 2G populated (first of which is not removable) and 1G unpopulated. PCI hole starts above 3G. I agree -m should mean one big unpluggable slot. So in the new proposal,-device dimm populated=true means a hot-removable dimm that has already been hotplugged. Yes. A question here is when exactly should the initial hot-add event for this dimm be played? If the relevant OSPM has not yet been initialized (e.g. acpi_memhotplug module in a linux guest needs to be loaded), the guest may not see the event. This is a general issue of course, but with initially populated hot-removable dimms it may be a bigger issue. Can ospm acpi initialization be detected? Or maybe you are suggesting populated=true is part of initial memory (i.e. not hot-added, but still hot-removable). Though in that case guestOS may use it for bootmem allocations, making hot-remove more likely to fail at the memory offlining stage. If memory slot is populated even before OSPM is started BIOS will detect that by reading mem_sts and will create e820 map appropriately. OSPM will detect it by evaluating _STA during boot. This is not unique for memory hot-plug. Any hotpluggable device have the same issue. -- Gleb.
Re: [Qemu-devel] copy benchmarks onto qemu
On Tue, Apr 24, 2012 at 02:09, Xin Tong xerox.time.t...@gmail.com wrote: I am not too sure what you mean by raw image. what i have is an *.img file that is bootable by QEMU. will kpartx work ? try to use qemu-img info to find what format the file uses... -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com
Re: [Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()
On Mon, Apr 23, 2012 at 7:01 PM, Luiz Capitulino lcapitul...@redhat.com wrote:
On Mon, 23 Apr 2012 17:47:09 +0200
Paolo Bonzini pbonz...@redhat.com wrote:
Il 23/04/2012 17:39, Stefan Hajnoczi ha scritto:
There are at least two different errors that can occur in
block_job_set_speed(): the job might not support setting speeds or the
value might be invalid.
Use the Error mechanism to report the error where it occurs. This patch
adds the new BlockJobSpeedInvalid QError. The error is specific to
block job speeds so we can add a speed argument to block-stream in the
future and clearly identify the invalid parameter.
Cc: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
block.c | 17 ++---
block/stream.c | 6 +++---
block_int.h | 5 +++--
blockdev.c | 4 +---
qapi-schema.json | 1 +
qerror.c | 4
qerror.h | 3 +++
7 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index 528b696..7056d8c 100644
--- a/block.c
+++ b/block.c
@@ -4103,18 +4103,21 @@ void block_job_complete(BlockJob *job, int ret)
bdrv_set_in_use(bs, 0);
}
-int block_job_set_speed(BlockJob *job, int64_t value)
+void block_job_set_speed(BlockJob *job, int64_t value, Error **errp)
{
- int rc;
+ Error *local_error = NULL;
if (!job-job_type-set_speed) {
- return -ENOTSUP;
+ error_set(errp, QERR_NOT_SUPPORTED);
+ return;
}
- rc = job-job_type-set_speed(job, value);
- if (rc == 0) {
- job-speed = value;
+ job-job_type-set_speed(job, value, local_error);
+ if (error_is_set(local_error)) {
+ error_propagate(errp, local_error);
+ return;
}
- return rc;
+
+ job-speed = value;
I don't think this is the right place to add Error handling. By doing
it at QAPI entry points we can reuse an existing error such as
QERR_INVALID_PARAMETER_VALUE.
I think the place were we call error_set() isn't a problem. Actually, the
Error
object was specifically designed to be used from qemu depths and be
propagated up.
Now:
If we need to introduce a specific error for each parameter type, we
will end up with N errors per function.
I agree with that, QError design induces people to add new errors... Why can't
you use one of the invalid parameter errors we have?
The error is specific to
block job speeds so we can add a speed argument to block-stream in the
future and clearly identify the invalid parameter.
I added the new error to avoid having to change the InvalidParameter
'name' field. It becomes ugly because we have:
block-job-set-speed device value
block-stream device [speed] [base]
Notice that the parameter is called 'value' in block-job-set-speed and
'speed' in block-stream. Therefore we can't just propagate a single
InvalidParameter name='value' error up from block-stream.
This means that QMP commands will need to change the error to use the
correct name :(. It's doable, but ugly and a bit more complex.
Thoughts?
Stefan
Re: [Qemu-devel] [PATCH 00/15] QOM'ify x86 CPU, part 2: properties
Am 19.04.2012 20:27, schrieb Eduardo Habkost: By the way, do you still plan to make cpudefs register new classes/types? I remember that you did that on a previous series. Generally I do, yes. However the CPU QOM'ification is not making as much progress as I would've liked, specifically there's still five targets left in my queue for base QOM'ification for 1.1. This series, x86 part 2, goes further and prepares properties a) for general inspection and modification (but without having the CPU exposed as a child yet), b) for use in instantiating subclasses in part 3. It was intended for 1.1. Question is, do we want CPU subclasses for 1.1? ARM has them now but we won't manage to unify this across targets in time for the Hard Freeze. For ARM there was a dislike of the ARMCPUClass-based approach and a preference towards hardcoding things imperatively in initfns. I don't see how that would work for cpudef, so I would stick with extending X86CPUClass. There was a dislike of duplicating fields between X86CPUInfo and X86CPUClass. However to avoid incurring a sub-struct access the only way would be to move the field definitions to a macro and use it in both locations. Further opinions or suggestions welcome. I've also not yet seen a discussion whether we need to allow modifying built-in classes via cpudef or whether adding new classes is sufficient. I had implemented both in my RFC but would prefer the latter if there is agreement. Is it possible to have property get/setters for ObjectClass QOM objects, too? It would be interesting to use QOM properties for the cpudef fields as well (it would make the work of defining boolean feature fields much simpler). No. Classes do not have property infrastructure and I personally see them as immutable. cpudef is independent of classes though, so you could invent a new mechanism to set/unset features, whatever the backend they'll be written to. Reviewed-by: Eduardo Habkost ehabk...@redhat.com Thanks, will send out v2 shortly with the requested changes. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] Generating cachegrind output with qemu
Dear Stefan, Thanks for your quick answer, I'll look into it. Regards, Gabor Wacha 2012.04.24. 6:33, Stefan Weil s...@weilnetz.de ezt írta: Am 23.04.2012 18:18, schrieb Wacha Gábor: Dear developers, I am a Hungarian student trying to use qemu for profiling bare metal ARM programs for my student research. On the following page, it is mentioned that one can generate cachegrind output with qemu: http://www.monstr.eu/wiki/**doku.php?id=qemu:qemu#run_**with_cachegrindhttp://www.monstr.eu/wiki/doku.php?id=qemu:qemu#run_with_cachegrindUnfortunately it does not work (altough I checked out the sources from the mentioned git repository), and I found out (using find and grep utilities), that cachegrind is not even mentioned in the qemu sources. My question is: does this feature exist? If the answer is yes, how can I use it? (And I am sorry for my English.) Thanks, Gabor Wacha Your English is good - much better than my Hungarian :-) Standard QEMU cannot produce cachegrind output. According to this mail, a derived version of QEMU supports cachegrind output for CRIS and MicroBlaze emulation: http://lists.gnu.org/archive/**html/qemu-devel/2009-06/**msg01415.htmlhttp://lists.gnu.org/archive/html/qemu-devel/2009-06/msg01415.html I cc'ed the author, Edgar Iglesias. Here are the sources: http://repo.or.cz/w/qemu/cris-**port.githttp://repo.or.cz/w/qemu/cris-port.git Cachegrind for ARM emulation is not supported and would have to be implemented in QEMU. Profiling could also be done by using the TCG interpreter and extending the interpreter main loop tcg_qemu_tb_exec in tci.c. Regards, Stefan Weil
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina phrd...@redhat.com wrote:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
s/IDE//
It's unrelated to IDE.
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+ /* This is needed for driver to detect there is no media in drive */
+ if (!bdrv_is_inserted(drv-bs))
+ return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
Re: [Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()
Il 24/04/2012 10:49, Stefan Hajnoczi ha scritto: The error is specific to block job speeds so we can add a speed argument to block-stream in the future and clearly identify the invalid parameter. I added the new error to avoid having to change the InvalidParameter 'name' field. It becomes ugly because we have: block-job-set-speed device value block-stream device [speed] [base] Notice that the parameter is called 'value' in block-job-set-speed and 'speed' in block-stream. Therefore we can't just propagate a single InvalidParameter name='value' error up from block-stream. This means that QMP commands will need to change the error to use the correct name :(. It's doable, but ugly and a bit more complex. Well, we still have time to change block-job-set-speed. Ugly, but doable. Paolo
Re: [Qemu-devel] [PATCH 00/16] QEMU vhost-scsi support
On Tue, Apr 24, 2012 at 8:05 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 24/04/2012 06:21, ronnie sahlberg ha scritto: Hi Stefan, A little bit off-topic but When you design the proper place and API to plug virt-scsi into an external SCSI parser outside of qemu like the target in the kernel ... It would be very nice if one could also plug virt-scsi into libiscsi and pass the CDBs straight to the remote iSCSI target too. Keep some thoughts on virt-scsi + libiscsi integration. Yes, that makes a lot of sense. It's a bit harder than scsi-generic but we do want to get there. Yep. I think previously there was discussion about a libiscsi SCSIDevice so that guest SCSI commands can be sent to libiscsi LUNs without going through the QEMU block layer. (Another way to pass arbitrary SCSI commands to libiscsi is by hooking up .bdrv_aio_ioctl() with SG_IO scsi-generic compatible code in block/iscsi.c.) I think what you're describing now is one level higher: the libiscsi target *is* the virtual SCSI target and virtio-scsi just ships off commands to it? This would be like a SCSIBus that is handle by libiscsi target instead of emulated in QEMU. Stefan
[Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization, and
true after, both for multiport aware and non multiport aware drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t
*config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
static void virtio_serial_save(QEMUFile *f, void *opaque)
{
VirtIOSerial *s = opaque;
@@ -798,14 +818,6 @@ static int virtser_port_qdev_init(DeviceState *qdev)
return ret;
}
-if (!use_multiport(port-vser)) {
-/*
- * Allow writes to guest in this case; we have no way of
- * knowing if a guest port is connected.
- */
-port-guest_connected = true;
-}
-
port-elem.out_num = 0;
QTAILQ_INSERT_TAIL(port-vser-ports, port, next);
@@ -905,6 +917,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev,
virtio_serial_conf *conf)
vser-vdev.get_features = get_features;
vser-vdev.get_config = get_config;
vser-vdev.set_config = set_config;
+vser-vdev.set_status = set_status;
vser-qdev = dev;
--
1.7.7.6
[Qemu-devel] [PATCH v2 09/15] target-i386: Add property getter for CPU model
Signed-off-by: Andreas Färber afaer...@suse.de
---
target-i386/cpu.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9479717..643289f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -640,6 +640,18 @@ static void x86_cpuid_version_set_family(Object *obj,
Visitor *v, void *opaque,
}
}
+static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+int64_t value;
+
+value = (env-cpuid_version 4) 0xf;
+value |= ((env-cpuid_version 16) 0xf) 4;
+visit_type_int(v, value, name, errp);
+}
+
static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
@@ -1557,7 +1569,7 @@ static void x86_cpu_initfn(Object *obj)
x86_cpuid_version_get_family,
x86_cpuid_version_set_family, NULL, NULL, NULL);
object_property_add(obj, model, int,
-NULL,
+x86_cpuid_version_get_model,
x86_cpuid_version_set_model, NULL, NULL, NULL);
object_property_add(obj, stepping, int,
NULL,
--
1.7.7
[Qemu-devel] [PATCH v2 12/15] target-i386: Introduce level property for X86CPU
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 38 +-
1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 490db76..f2df8d0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -711,6 +711,39 @@ static void x86_cpuid_version_set_stepping(Object *obj,
Visitor *v,
env-cpuid_version |= value 0xf;
}
+static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu-env.cpuid_level;
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, value, name, errp);
+}
+
+static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = UINT32_MAX;
+int64_t value;
+
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
+cpu-env.cpuid_level = value;
+}
+
static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
@@ -1037,7 +1070,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
}
env-cpuid_vendor_override = def-vendor_override;
-env-cpuid_level = def-level;
+object_property_set_int(OBJECT(cpu), def-level, level, error);
object_property_set_int(OBJECT(cpu), def-family, family, error);
object_property_set_int(OBJECT(cpu), def-model, model, error);
object_property_set_int(OBJECT(cpu), def-stepping, stepping, error);
@@ -1601,6 +1634,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, stepping, int,
x86_cpuid_version_get_stepping,
x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add(obj, level, int,
+x86_cpuid_get_level,
+x86_cpuid_set_level, NULL, NULL, NULL);
object_property_add_str(obj, model-id,
x86_cpuid_get_model_id,
x86_cpuid_set_model_id, NULL);
--
1.7.7
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdinaphrd...@redhat.com wrote:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
s/IDE//
It's unrelated to IDE.
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
[Qemu-devel] [PATCH v2 04/15] target-i386: Add family property to X86CPU
Add the property early in the initfn so that it can be used in helpers
such as mce_init().
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
[AF: Add an error_free(), spotted by Michael Roth]
---
target-i386/cpu.c | 39 ++-
1 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d30185b..ebe9c7e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -27,6 +27,8 @@
#include qemu-option.h
#include qemu-config.h
+#include qapi/qapi-visit-core.h
+
#include hyperv.h
/* feature flags taken from Intel Processor Identification and the CPUID
@@ -597,13 +599,30 @@ static int check_features_against_host(x86_def_t
*guest_def)
return rv;
}
-static void x86_cpuid_version_set_family(CPUX86State *env, int family)
+static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+const int64_t min = 0;
+const int64_t max = 0xff + 0xf;
+int64_t value;
+
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
env-cpuid_version = ~0xff00f00;
-if (family 0x0f) {
-env-cpuid_version |= 0xf00 | ((family - 0x0f) 20);
+if (value 0x0f) {
+env-cpuid_version |= 0xf00 | ((value - 0x0f) 20);
} else {
-env-cpuid_version |= family 8;
+env-cpuid_version |= value 8;
}
}
@@ -911,6 +930,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
{
CPUX86State *env = cpu-env;
x86_def_t def1, *def = def1;
+Error *error = NULL;
memset(def, 0, sizeof(*def));
@@ -927,7 +947,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
}
env-cpuid_vendor_override = def-vendor_override;
env-cpuid_level = def-level;
-x86_cpuid_version_set_family(env, def-family);
+object_property_set_int(OBJECT(cpu), def-family, family, error);
x86_cpuid_version_set_model(env, def-model);
x86_cpuid_version_set_stepping(env, def-stepping);
env-cpuid_features = def-features;
@@ -952,6 +972,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_svm_features = TCG_SVM_FEATURES;
}
x86_cpuid_set_model_id(env, def-model_id);
+if (error_is_set(error)) {
+error_free(error);
+return -1;
+}
return 0;
}
@@ -1476,6 +1500,11 @@ static void x86_cpu_initfn(Object *obj)
CPUX86State *env = cpu-env;
cpu_exec_init(env);
+
+object_property_add(obj, family, int,
+NULL,
+x86_cpuid_version_set_family, NULL, NULL, NULL);
+
env-cpuid_apic_id = env-cpu_index;
mce_init(cpu);
}
--
1.7.7
[Qemu-devel] [PATCH v2 14/15] target-i386: Prepare vendor property for X86CPU
Using it now would incur converting the three x86_def_t vendor words
into a string for object_property_set_str(), then back to three words
in the vendor setter.
The built-in CPU definitions use numeric preprocessor defines to
initialize the three words in a charset-safe way, so do not change the
fields to char[12] just to use the setter.
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 44
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89a1855..540b3df 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -777,6 +777,47 @@ static void x86_cpuid_set_xlevel(Object *obj, Visitor *v,
void *opaque,
cpu-env.cpuid_xlevel = value;
}
+static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+char *value;
+int i;
+
+value = (char *)g_malloc(12 + 1);
+for (i = 0; i 4; i++) {
+value[i] = env-cpuid_vendor1 (8 * i);
+value[i + 4] = env-cpuid_vendor2 (8 * i);
+value[i + 8] = env-cpuid_vendor3 (8 * i);
+}
+value[12] = '\0';
+return value;
+}
+
+static void x86_cpuid_set_vendor(Object *obj, const char *value,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+int i;
+
+if (strlen(value) != 12) {
+error_set(errp, QERR_PROPERTY_VALUE_BAD, ,
+ vendor, value);
+return;
+}
+
+env-cpuid_vendor1 = 0;
+env-cpuid_vendor2 = 0;
+env-cpuid_vendor3 = 0;
+for (i = 0; i 4; i++) {
+env-cpuid_vendor1 |= ((uint8_t)value[i]) (8 * i);
+env-cpuid_vendor2 |= ((uint8_t)value[i + 4]) (8 * i);
+env-cpuid_vendor3 |= ((uint8_t)value[i + 8]) (8 * i);
+}
+env-cpuid_vendor_override = 1;
+}
+
static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
@@ -1673,6 +1714,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, xlevel, int,
x86_cpuid_get_xlevel,
x86_cpuid_set_xlevel, NULL, NULL, NULL);
+object_property_add_str(obj, vendor,
+x86_cpuid_get_vendor,
+x86_cpuid_set_vendor, NULL);
object_property_add_str(obj, model-id,
x86_cpuid_get_model_id,
x86_cpuid_set_model_id, NULL);
--
1.7.7
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the
drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no
media in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults
and both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdina phrd...@redhat.com
Signed-off-by: Michal Novotny minov...@redhat.com
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs, nb_heads, max_track,
last_sect, drv-drive, drive, rate);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags = ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
Kevin
Re: [Qemu-devel] [PATCH] spice: require spice-protocol = 0.8.1
On 04/23/12 15:52, Peter Maydell wrote: Ping? This patch doesn't seem to have made it into master yet and I don't think it was in the last spice pullreq... Somehow overlooked it in my inbox. Added to the spice patch queue now. Thanks for the reminder, Gerd
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdinaphrd...@redhat.com
Signed-off-by: Michal Novotnyminov...@redhat.com
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs,nb_heads,max_track,
last_sect, drv-drive,drive,rate);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
This modification is needed for floppy driver in guest to detect floppy drive.
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags= ~FDISK_DBL_SIDES;
}
+
}
This code is needed to set up default drive geometry so guest can detect
floppy drive controller.
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
And this is needed for all cases if there is no media in drive. Code in
fdctrl_change_cb() is needed only for detect that there is no media when
you try to mount it (linux guest) or open it (windows guest).
Kevin
[Qemu-devel] [PULL] virtio-serial: fix probing for features before driver init
Hi, Please pull for a virtio-serial fix. The following changes since commit 3c30dd5a68e9fee6af67cfd0d14ed7520820f36a: target-arm: Move reset handling to arm_cpu_reset (2012-04-21 18:13:22 +) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git master Alon Levy (1): virtio-serial-bus: fix guest_connected init before driver init hw/virtio-serial-bus.c | 29 + 1 files changed, 21 insertions(+), 8 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH v2 15/15] target-i386: Introduce tsc-frequency property for X86CPU
Use Hz as unit.
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 37 -
1 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 540b3df..64fd903 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -857,6 +857,37 @@ static void x86_cpuid_set_model_id(Object *obj, const char
*model_id,
}
}
+static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu-env.tsc_khz * 1000;
+visit_type_int(v, value, name, errp);
+}
+
+static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = INT_MAX;
+int64_t value;
+
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
+cpu-env.tsc_khz = value / 1000;
+}
+
static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
{
unsigned int i;
@@ -1157,7 +1188,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_svm_features = def-svm_features;
env-cpuid_ext4_features = def-ext4_features;
env-cpuid_xlevel2 = def-xlevel2;
-env-tsc_khz = def-tsc_khz;
+object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
+tsc-frequency, error);
if (!kvm_enabled()) {
env-cpuid_features = TCG_FEATURES;
env-cpuid_ext_features = TCG_EXT_FEATURES;
@@ -1720,6 +1752,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add_str(obj, model-id,
x86_cpuid_get_model_id,
x86_cpuid_set_model_id, NULL);
+object_property_add(obj, tsc-frequency, int,
+x86_cpuid_get_tsc_freq,
+x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
env-cpuid_apic_id = env-cpu_index;
mce_init(cpu);
--
1.7.7
[Qemu-devel] [PATCH v2 07/15] target-i386: Add model-id property to X86CPU
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2f843be..1b8053a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -673,8 +673,11 @@ static void x86_cpuid_version_set_stepping(Object *obj,
Visitor *v,
env-cpuid_version |= value 0xf;
}
-static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
+static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
+ Error **errp)
{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
int c, len, i;
if (model_id == NULL) {
@@ -1006,7 +1009,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_ext3_features = TCG_EXT3_FEATURES;
env-cpuid_svm_features = TCG_SVM_FEATURES;
}
-x86_cpuid_set_model_id(env, def-model_id);
+object_property_set_str(OBJECT(cpu), def-model_id, model-id, error);
if (error_is_set(error)) {
error_free(error);
return -1;
@@ -1545,6 +1548,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, stepping, int,
NULL,
x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add_str(obj, model-id,
+NULL,
+x86_cpuid_set_model_id, NULL);
env-cpuid_apic_id = env-cpu_index;
mce_init(cpu);
--
1.7.7
[Qemu-devel] [PATCH v2 02/15] target-i386: Pass X86CPU to cpu_x86_register()
Avoids an x86_env_get_cpu() call there, to work with QOM properties.
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c|3 ++-
target-i386/cpu.h|2 +-
target-i386/helper.c |2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 80c1ca5..e95a1d8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -907,8 +907,9 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf,
const char *optarg)
}
}
-int cpu_x86_register (CPUX86State *env, const char *cpu_model)
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
{
+CPUX86State *env = cpu-env;
x86_def_t def1, *def = def1;
memset(def, 0, sizeof(*def));
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 4bb4592..b5b9a50 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -901,7 +901,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
-int cpu_x86_register (CPUX86State *env, const char *cpu_model);
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
void cpu_clear_apic_feature(CPUX86State *env);
void host_cpuid(uint32_t function, uint32_t count,
uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 87954f0..0b22582 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1176,7 +1176,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
cpu_set_debug_excp_handler(breakpoint_handler);
#endif
}
-if (cpu_x86_register(env, cpu_model) 0) {
+if (cpu_x86_register(cpu, cpu_model) 0) {
object_delete(OBJECT(cpu));
return NULL;
}
--
1.7.7
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization, and
true after, both for multiport aware and non multiport aware drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Logic also changed fron Alon's patch. Why?
Yes, forgot to mention that here.
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const
uint8_t *config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
Weird. Don't you want to set guest_connected = false when driver
is unloaded?
This is what Alon's patch did:
port-guest_connected = status VIRTIO_CONFIG_S_DRIVER_OK;
Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).
And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.
Is that something we should do? It's obviously correct to do that.
Will it affect anything? I doubt it. But needs a separate patch and
discussion for that change.
Amit
[Qemu-devel] [PATCH v2 03/15] target-i386: Add range check for -cpu , family=x
A family field value of 0xf and extended family field value of 0xff is
the maximum representable unsigned family number.
All other CPUID property values are bounds-checked, so add a check here
for symmetry before we adopt it in a property setter.
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e95a1d8..d30185b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -693,7 +693,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
if (!strcmp(featurestr, family)) {
char *err;
numvalue = strtoul(val, err, 0);
-if (!*val || *err) {
+if (!*val || *err || numvalue 0xff + 0xf) {
fprintf(stderr, bad numerical value %s\n, val);
goto error;
}
--
1.7.7
[Qemu-devel] [PATCH v2 00/15] QOM'ify x86 CPU, part 2: properties
Hello, This series introduces some QOM properties for X86CPU, so that our built-in init code exercises the same code paths as QMP, as suggested by Eduardo: * family, * model, * stepping and * model-id (rather than model_id) This QOM'ifies my previously introduced helper functions, adding getters. In the same spirit I've also introduced numeric QOM properties for: * level * xlevel * tsc-frequency (rather than tsc_freq) Further I've prepared one QOM property that's currently unused: * vendor (converting three words to string and back seemed too much overhead) By constrast, the HyperV -cpu property hv_spinlocks and flags hv_relaxed and hv_vapic do not seem to be per-CPU properties. v2 fixes minor issues and if I can get an Acked-by or Reviewed-by for 09/15 then I'll send a PULL tomorrow. Available from: git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-prop.v2 https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-prop.v2 Regards, Andreas Cc: Anthony Liguori anth...@codemonkey.ws Cc: Jan Kiszka jan.kis...@siemens.com Cc: Igor Mammedov imamm...@redhat.com Cc: Liu Jinsong jinsong@intel.com Cc: Lai Jiangshan la...@cn.fujitsu.com Cc: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com Cc: Eduardo Habkost ehabk...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Vadim Rozenfeld vroze...@redhat.com Andreas Färber (15): target-i386: Fix x86_cpuid_set_model_id() target-i386: Pass X86CPU to cpu_x86_register() target-i386: Add range check for -cpu ,family=x target-i386: Add family property to X86CPU target-i386: Add model property to X86CPU target-i386: Add stepping property to X86CPU target-i386: Add model-id property to X86CPU target-i386: Add property getter for CPU family target-i386: Add property getter for CPU model target-i386: Add property getter for CPU stepping target-i386: Add property getter for CPU model-id target-i386: Introduce level property for X86CPU target-i386: Introduce xlevel property for X86CPU target-i386: Prepare vendor property for X86CPU target-i386: Introduce tsc-frequency property for X86CPU target-i386/cpu.c| 320 +++--- target-i386/cpu.h|2 +- target-i386/helper.c |2 +- 3 files changed, 304 insertions(+), 20 deletions(-) -- 1.7.7
[Qemu-devel] [PATCH v2 05/15] target-i386: Add model property to X86CPU
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 26 +++---
1 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ebe9c7e..3a8141b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -626,10 +626,27 @@ static void x86_cpuid_version_set_family(Object *obj,
Visitor *v, void *opaque,
}
}
-static void x86_cpuid_version_set_model(CPUX86State *env, int model)
+static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+const int64_t min = 0;
+const int64_t max = 0xff;
+int64_t value;
+
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
env-cpuid_version = ~0xf00f0;
-env-cpuid_version |= ((model 0xf) 4) | ((model 4) 16);
+env-cpuid_version |= ((value 0xf) 4) | ((value 4) 16);
}
static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
@@ -948,7 +965,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_vendor_override = def-vendor_override;
env-cpuid_level = def-level;
object_property_set_int(OBJECT(cpu), def-family, family, error);
-x86_cpuid_version_set_model(env, def-model);
+object_property_set_int(OBJECT(cpu), def-model, model, error);
x86_cpuid_version_set_stepping(env, def-stepping);
env-cpuid_features = def-features;
env-cpuid_ext_features = def-ext_features;
@@ -1504,6 +1521,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, family, int,
NULL,
x86_cpuid_version_set_family, NULL, NULL, NULL);
+object_property_add(obj, model, int,
+NULL,
+x86_cpuid_version_set_model, NULL, NULL, NULL);
env-cpuid_apic_id = env-cpu_index;
mce_init(cpu);
--
1.7.7
[Qemu-devel] [PATCH v2 10/15] target-i386: Add property getter for CPU stepping
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 643289f..f1d3827 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -675,6 +675,18 @@ static void x86_cpuid_version_set_model(Object *obj,
Visitor *v, void *opaque,
env-cpuid_version |= ((value 0xf) 4) | ((value 4) 16);
}
+static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+int64_t value;
+
+value = env-cpuid_version 0xf;
+visit_type_int(v, value, name, errp);
+}
+
static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1572,7 +1584,7 @@ static void x86_cpu_initfn(Object *obj)
x86_cpuid_version_get_model,
x86_cpuid_version_set_model, NULL, NULL, NULL);
object_property_add(obj, stepping, int,
-NULL,
+x86_cpuid_version_get_stepping,
x86_cpuid_version_set_stepping, NULL, NULL, NULL);
object_property_add_str(obj, model-id,
NULL,
--
1.7.7
[Qemu-devel] [PATCH v2 06/15] target-i386: Add stepping property to X86CPU
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 27 ---
1 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3a8141b..2f843be 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -649,10 +649,28 @@ static void x86_cpuid_version_set_model(Object *obj,
Visitor *v, void *opaque,
env-cpuid_version |= ((value 0xf) 4) | ((value 4) 16);
}
-static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
+static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+const int64_t min = 0;
+const int64_t max = 0xf;
+int64_t value;
+
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
env-cpuid_version = ~0xf;
-env-cpuid_version |= stepping 0xf;
+env-cpuid_version |= value 0xf;
}
static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
@@ -966,7 +984,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_level = def-level;
object_property_set_int(OBJECT(cpu), def-family, family, error);
object_property_set_int(OBJECT(cpu), def-model, model, error);
-x86_cpuid_version_set_stepping(env, def-stepping);
+object_property_set_int(OBJECT(cpu), def-stepping, stepping, error);
env-cpuid_features = def-features;
env-cpuid_ext_features = def-ext_features;
env-cpuid_ext2_features = def-ext2_features;
@@ -1524,6 +1542,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, model, int,
NULL,
x86_cpuid_version_set_model, NULL, NULL, NULL);
+object_property_add(obj, stepping, int,
+NULL,
+x86_cpuid_version_set_stepping, NULL, NULL, NULL);
env-cpuid_apic_id = env-cpu_index;
mce_init(cpu);
--
1.7.7
[Qemu-devel] [PATCH v2 11/15] target-i386: Add property getter for CPU model-id
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 17 -
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f1d3827..490db76 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -711,6 +711,21 @@ static void x86_cpuid_version_set_stepping(Object *obj,
Visitor *v,
env-cpuid_version |= value 0xf;
}
+static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+char *value;
+int i;
+
+value = g_malloc(48 + 1);
+for (i = 0; i 48; i++) {
+value[i] = env-cpuid_model[i 2] (8 * (i 3));
+}
+value[48] = '\0';
+return value;
+}
+
static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
Error **errp)
{
@@ -1587,7 +1602,7 @@ static void x86_cpu_initfn(Object *obj)
x86_cpuid_version_get_stepping,
x86_cpuid_version_set_stepping, NULL, NULL, NULL);
object_property_add_str(obj, model-id,
-NULL,
+x86_cpuid_get_model_id,
x86_cpuid_set_model_id, NULL);
env-cpuid_apic_id = env-cpu_index;
--
1.7.7
Re: [Qemu-devel] [PATCH v2 00/15] QOM'ify x86 CPU, part 2: properties
Am 24.04.2012 11:33, schrieb Andreas Färber: Hello, This series introduces some QOM properties for X86CPU, so that our built-in init code exercises the same code paths as QMP, as suggested by Eduardo: * family, * model, * stepping and * model-id (rather than model_id) This QOM'ifies my previously introduced helper functions, adding getters. In the same spirit I've also introduced numeric QOM properties for: * level * xlevel * tsc-frequency (rather than tsc_freq) Further I've prepared one QOM property that's currently unused: * vendor (converting three words to string and back seemed too much overhead) By constrast, the HyperV -cpu property hv_spinlocks and flags hv_relaxed and hv_vapic do not seem to be per-CPU properties. v2 fixes minor issues and if I can get an Acked-by or Reviewed-by for 09/15 then I'll send a PULL tomorrow. Available from: git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-prop.v2 https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-prop.v2 Regards, Andreas Cc: Anthony Liguori anth...@codemonkey.ws Cc: Jan Kiszka jan.kis...@siemens.com Cc: Igor Mammedov imamm...@redhat.com Cc: Liu Jinsong jinsong@intel.com Cc: Lai Jiangshan la...@cn.fujitsu.com Cc: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com Cc: Eduardo Habkost ehabk...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Vadim Rozenfeld vroze...@redhat.com Sorry, forgot the change log: v1 - v2: * Added error_free() in cpu_x86_register() (spotted by Michael). * Added missing shift in model property getter (spotted by Michael). * Extended some commit messages. /-F Andreas Färber (15): target-i386: Fix x86_cpuid_set_model_id() target-i386: Pass X86CPU to cpu_x86_register() target-i386: Add range check for -cpu ,family=x target-i386: Add family property to X86CPU target-i386: Add model property to X86CPU target-i386: Add stepping property to X86CPU target-i386: Add model-id property to X86CPU target-i386: Add property getter for CPU family target-i386: Add property getter for CPU model target-i386: Add property getter for CPU stepping target-i386: Add property getter for CPU model-id target-i386: Introduce level property for X86CPU target-i386: Introduce xlevel property for X86CPU target-i386: Prepare vendor property for X86CPU target-i386: Introduce tsc-frequency property for X86CPU target-i386/cpu.c| 320 +++--- target-i386/cpu.h|2 +- target-i386/helper.c |2 +- 3 files changed, 304 insertions(+), 20 deletions(-) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH v2 01/15] target-i386: Fix x86_cpuid_set_model_id()
Don't assume zeroed cpuid_model[] fields.
This doesn't break anything yet but QOM properties should be able to set
the value to something else without setting an intermediate zero string.
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3df53ca..80c1ca5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -627,6 +627,9 @@ static void x86_cpuid_set_model_id(CPUX86State *env, const
char *model_id)
model_id = ;
}
len = strlen(model_id);
+for (i = 0; i 12; i++) {
+env-cpuid_model[i] = 0;
+}
for (i = 0; i 48; i++) {
if (i = len) {
c = '\0';
--
1.7.7
[Qemu-devel] [PATCH v2 08/15] target-i386: Add property getter for CPU family
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 16 +++-
1 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1b8053a..9479717 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -599,6 +599,20 @@ static int check_features_against_host(x86_def_t
*guest_def)
return rv;
}
+static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+int64_t value;
+
+value = (env-cpuid_version 8) 0xf;
+if (value == 0xf) {
+value += (env-cpuid_version 20) 0xff;
+}
+visit_type_int(v, value, name, errp);
+}
+
static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
@@ -1540,7 +1554,7 @@ static void x86_cpu_initfn(Object *obj)
cpu_exec_init(env);
object_property_add(obj, family, int,
-NULL,
+x86_cpuid_version_get_family,
x86_cpuid_version_set_family, NULL, NULL, NULL);
object_property_add(obj, model, int,
NULL,
--
1.7.7
[Qemu-devel] [PATCH v2 13/15] target-i386: Introduce xlevel property for X86CPU
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 38 +-
1 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f2df8d0..89a1855 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -744,6 +744,39 @@ static void x86_cpuid_set_level(Object *obj, Visitor *v,
void *opaque,
cpu-env.cpuid_level = value;
}
+static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu-env.cpuid_xlevel;
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, value, name, errp);
+}
+
+static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = UINT32_MAX;
+int64_t value;
+
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value min || value max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, ,
+ name ? name : null, value, min, max);
+return;
+}
+
+cpu-env.cpuid_xlevel = value;
+}
+
static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
@@ -1078,7 +,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_ext_features = def-ext_features;
env-cpuid_ext2_features = def-ext2_features;
env-cpuid_ext3_features = def-ext3_features;
-env-cpuid_xlevel = def-xlevel;
+object_property_set_int(OBJECT(cpu), def-xlevel, xlevel, error);
env-cpuid_kvm_features = def-kvm_features;
env-cpuid_svm_features = def-svm_features;
env-cpuid_ext4_features = def-ext4_features;
@@ -1637,6 +1670,9 @@ static void x86_cpu_initfn(Object *obj)
object_property_add(obj, level, int,
x86_cpuid_get_level,
x86_cpuid_set_level, NULL, NULL, NULL);
+object_property_add(obj, xlevel, int,
+x86_cpuid_get_xlevel,
+x86_cpuid_set_xlevel, NULL, NULL, NULL);
object_property_add_str(obj, model-id,
x86_cpuid_get_model_id,
x86_cpuid_set_model_id, NULL);
--
1.7.7
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization, and
true after, both for multiport aware and non multiport aware drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Logic also changed fron Alon's patch. Why?
Yes, forgot to mention that here.
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const
uint8_t *config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
Weird. Don't you want to set guest_connected = false when driver
is unloaded?
This is what Alon's patch did:
port-guest_connected = status VIRTIO_CONFIG_S_DRIVER_OK;
Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).
And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.
Is that something we should do? It's obviously correct to do that.
Will it affect anything? I doubt it. But needs a separate patch and
discussion for that change.
Amit
Let's not add code to preserve a bug so we can have a discussion. Let's
have a discussion right here and fix it properly.
BTW guest_connected is not cleared on reset either - a bug too?
--
MST
Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug
On 04/24/2012 10:21 AM, Gleb Natapov wrote: I was thinking about not having tons of 128MB slots, so we don't have a configuration that is far from reality. But maybe this thinking is too conservative. I think it is good interface to make memory that is specified with -m to be one big unpluggable slot, but slots defined with -device should start just above what -m specifies (after proper alignment). Memory hot-plug granularity is controlled by slot's size parameter. Agree. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 09:48 AM, Andreas Färber wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdinaphrd...@redhat.com
Signed-off-by: Michal Novotnyminov...@redhat.com
---
Please see http://wiki.qemu.org/Contribute/SubmitAPatch for hints on
writing a commit message for a patch: Subject should have a prefix, such
as fdc: Fix emulation for no media and the body should not contain
personal letter-style contents such as Hi, this is the patch. Any such
personal comments can go under the --- line where they are not committed
to git history.
Cc'ing floppy author and block maintainer.
Andreas
Thanks for correction, I'll send v2 patch with right commit message.
Pavel
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs,nb_heads,max_track,
last_sect, drv-drive,drive,rate);
-if (nb_heads != 0 max_track != 0 last_sect != 0) {
-FLOPPY_DPRINTF(User defined disk (%d %d %d),
+if (!bdrv_is_inserted(drv-bs)) {
+FLOPPY_DPRINTF(No media in drive\n);
+} else if (nb_heads != 0 max_track != 0 last_sect != 0) {
+FLOPPY_DPRINTF(User defined disk (%d %d %d)\n,
nb_heads - 1, max_track, last_sect);
} else {
FLOPPY_DPRINTF(Floppy disk (%d h %d t %d s) %s\n, nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
drv-drive = drive;
drv-media_rate = rate;
} else {
-FLOPPY_DPRINTF(No disk in drive\n);
+FLOPPY_DPRINTF(Drive disabled\n);
drv-last_sect = 0;
drv-max_track = 0;
drv-flags= ~FDISK_DBL_SIDES;
}
+
}
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
if (floppy) {
fdc_get_bs(fd, floppy);
for (i = 0; i 2; i++) {
-if (fd[i] bdrv_is_inserted(fd[i])) {
+/* If there is no media in a drive, we still have the drive
present */
+if (fd[i]) {
bdrv_get_floppy_geometry_hint(fd[i],nb_heads,max_track,
last_sect, FDRIVE_DRV_NONE,
fd_type[i],rate);
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization, and
true after, both for multiport aware and non multiport aware drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Logic also changed fron Alon's patch. Why?
Yes, forgot to mention that here.
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const
uint8_t *config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
Weird. Don't you want to set guest_connected = false when driver
is unloaded?
This is what Alon's patch did:
port-guest_connected = status VIRTIO_CONFIG_S_DRIVER_OK;
Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).
And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.
Is that something we should do? It's obviously correct to do that.
Will it affect anything? I doubt it. But needs a separate patch and
discussion for that change.
Let's not add code to preserve a bug so we can have a discussion. Let's
have a discussion right here and fix it properly.
It can be added to the series, but has to be a different patch. But I
don't think we should hold up this fix because we found other bugs.
BTW guest_connected is not cleared on reset either - a bug too?
Yes, I think we'll just have to scan the list of things that we want
zero'ed at start.
Better to use a non-zeroed memory alloc than zero'ed, we can at least
be careful to set and reset values explicitly rather than invite such
bugs.
Amit
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On Tue, Apr 24, 2012 at 04:44:10PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization,
and
true after, both for multiport aware and non multiport aware drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Logic also changed fron Alon's patch. Why?
Yes, forgot to mention that here.
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const
uint8_t *config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
Weird. Don't you want to set guest_connected = false when driver
is unloaded?
This is what Alon's patch did:
port-guest_connected = status VIRTIO_CONFIG_S_DRIVER_OK;
Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).
And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.
Is that something we should do? It's obviously correct to do that.
Will it affect anything? I doubt it. But needs a separate patch and
discussion for that change.
Let's not add code to preserve a bug so we can have a discussion. Let's
have a discussion right here and fix it properly.
It can be added to the series, but has to be a different patch. But I
don't think we should hold up this fix because we found other bugs.
Sure. So fix it for single port now and for multiport in a
separate patch. Alon's patch is a single line and it made sense, yours
differs in that you have added more code to implement a bug.
BTW guest_connected is not cleared on reset either - a bug too?
Yes, I think we'll just have to scan the list of things that we want
zero'ed at start.
Not at start, at reset.
Better to use a non-zeroed memory alloc than zero'ed, we can at least
be careful to set and reset values explicitly rather than invite such
bugs.
Amit
[Qemu-devel] [PATCH] xen_disk: remove syncwrite option
Use the BDRV_O_CACHE_* flags instead.
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
hw/xen_disk.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 4a6d89c..3e4a47b 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -48,7 +48,6 @@
/* - */
-static int syncwrite= 0;
static int batch_maps = 0;
static int max_requests = 32;
@@ -189,15 +188,10 @@ static int ioreq_parse(struct ioreq *ioreq)
ioreq-presync = 1;
return 0;
}
-if (!syncwrite) {
-ioreq-postsync = 1;
-}
+ioreq-postsync = 1;
/* fall through */
case BLKIF_OP_WRITE:
ioreq-prot = PROT_READ; /* from memory */
-if (syncwrite) {
-ioreq-postsync = 1;
-}
break;
default:
xen_be_printf(blkdev-xendev, 0, error: unknown operation (%d)\n,
--
1.7.2.5
[Qemu-devel] [PULL] xen patches
Hi Anthony, please pull: git://xenbits.xen.org/people/sstabellini/qemu-dm.git for_anthony it contains a couple of xen_disk improvements and Xen HVM reboot support. John V. Baboval (1): xen: Support guest reboots Stefano Stabellini (2): xen_disk: use bdrv_aio_flush instead of bdrv_flush xen_disk: implement BLKIF_OP_FLUSH_DISKCACHE, remove BLKIF_OP_WRITE_BARRIER hw/xen_common.h |2 +- hw/xen_disk.c | 21 ++--- xen-all.c | 18 +++--- 3 files changed, 22 insertions(+), 19 deletions(-) Thanks, Stefano
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
Am 24.04.2012 12:26, schrieb Amit Shah: On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: From: Alon Levy al...@redhat.com guest_connected should be false before guest driver initialization, and true after, both for multiport aware and non multiport aware drivers. Don't set it before the guest_features are available; instead use set_status which is called by io to VIRTIO_PCI_STATUS with VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. [Amit: Add comment, tweak summary] Logic also changed fron Alon's patch. Why? Yes, forgot to mention that here. Re here: FWIW I learnt to add these sort of comments before the new SoB, i.e. between mst's Acked-by and your Signed-off-by so that it is clear that he ack'ed the previous version given the logic change. In this case it's arguably irrelevant since there's only one SoB by you. Andreas Signed-off-by: Alon Levy al...@redhat.com Acked-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Amit Shah amit.s...@redhat.com -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2 01/15] target-i386: Fix x86_cpuid_set_model_id()
On 04/24/2012 11:33 AM, Andreas Färber wrote:
Don't assume zeroed cpuid_model[] fields.
This doesn't break anything yet but QOM properties should be able to set
the value to something else without setting an intermediate zero string.
Signed-off-by: Andreas Färberafaer...@suse.de
Reviewed-by: Eduardo Habkostehabk...@redhat.com
---
target-i386/cpu.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3df53ca..80c1ca5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -627,6 +627,9 @@ static void x86_cpuid_set_model_id(CPUX86State *env, const
char *model_id)
model_id = ;
}
len = strlen(model_id);
+for (i = 0; i 12; i++) {
+env-cpuid_model[i] = 0;
+}
It's not important, but why not to use memset here?
for (i = 0; i 48; i++) {
if (i= len) {
c = '\0';
--
-
Igor
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On Tue, Apr 24, 2012 at 10:46 AM, Pavel Hrdina phrd...@redhat.com wrote:
On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:
On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina phrd...@redhat.com wrote:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
s/IDE//
It's unrelated to IDE.
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+ /* This is needed for driver to detect there is no media in drive */
+ if (!bdrv_is_inserted(drv-bs))
+ return 1;
if (drv-media_changed) {
drv-media_changed = 0;
ret = 1;
Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?
Stefan
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm ,
for specification of DIR register. Bit7 is there as CHAN and in this bit is
saved information whether media is changed or not. This bit is set to true
while there is no media. And floppy driver is checking this bit to detect
media change or media missing.
I can't find anything from your link that says the bit is set while
there is no media.
Disk Change: 1 = disk changed since last command, 0 = disk not changed
The 82078 44 PIN CHMOS SINGLE-CHIP FLOPPY DISK CONTROLLER datasheet
(http://wiki.qemu.org/images/f/f0/29047403.pdf) doesn't give details
but suggests what you are saying. It says DSKCHG monitors the pin of
the same name and
reflects the opposite value seen on the disk cable. So perhaps this
is really a level-triggered is the drive empty? bit.
This is still pretty unclear to me. Also, if you need to implement
the level-triggered behavior, then can we remove/simplify/clean up the
existing media change code in hw/fdc.c?
Stefan
Re: [Qemu-devel] [PATCH v2 01/15] target-i386: Fix x86_cpuid_set_model_id()
Am 24.04.2012 13:32, schrieb Igor Mammedov:
On 04/24/2012 11:33 AM, Andreas Färber wrote:
Don't assume zeroed cpuid_model[] fields.
This doesn't break anything yet but QOM properties should be able to set
Should've read didn't. I sure hope it doesn't. :)
the value to something else without setting an intermediate zero string.
Signed-off-by: Andreas Färberafaer...@suse.de
Reviewed-by: Eduardo Habkostehabk...@redhat.com
---
target-i386/cpu.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3df53ca..80c1ca5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -627,6 +627,9 @@ static void x86_cpuid_set_model_id(CPUX86State
*env, const char *model_id)
model_id = ;
}
len = strlen(model_id);
+for (i = 0; i 12; i++) {
+env-cpuid_model[i] = 0;
+}
It's not important, but why not to use memset here?
I guess I was blinded by the for loop below. ;)
Will change it for the PULL if there's no other reason to resend.
Thanks for asking,
Andreas
for (i = 0; i 48; i++) {
if (i= len) {
c = '\0';
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On (Tue) 24 Apr 2012 [14:21:36], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 04:44:10PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:49:53], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 03:56:44PM +0530, Amit Shah wrote:
On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
From: Alon Levy al...@redhat.com
guest_connected should be false before guest driver initialization,
and
true after, both for multiport aware and non multiport aware
drivers.
Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
[Amit: Add comment, tweak summary]
Logic also changed fron Alon's patch. Why?
Yes, forgot to mention that here.
Signed-off-by: Alon Levy al...@redhat.com
Acked-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 29 +
1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev,
const uint8_t *config_data)
memcpy(config, config_data, sizeof(config));
}
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port !use_multiport(port-vser)
+ (status VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status. Such guests can only have a port at
id
+ * 0, so set guest_connected for such ports as soon as
guest
+ * is up.
+ */
+port-guest_connected = true;
+}
+}
+
Weird. Don't you want to set guest_connected = false when driver
is unloaded?
This is what Alon's patch did:
port-guest_connected = status VIRTIO_CONFIG_S_DRIVER_OK;
Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).
And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.
Is that something we should do? It's obviously correct to do that.
Will it affect anything? I doubt it. But needs a separate patch and
discussion for that change.
Let's not add code to preserve a bug so we can have a discussion. Let's
have a discussion right here and fix it properly.
It can be added to the series, but has to be a different patch. But I
don't think we should hold up this fix because we found other bugs.
Sure. So fix it for single port now and for multiport in a
separate patch. Alon's patch is a single line and it made sense, yours
differs in that you have added more code to implement a bug.
Disagree; it was not obvious in that patch; a single patch did more
thing than one, commit log didn't note it (it was a side-effect), and
the code was not easy to read.
With the patch I will propose, there won't be any differentiation for
the multiport and not multiport case. And for virtio-serial,
multiport is the more important use-case (i.e. all recent guests have
multiport support).
BTW guest_connected is not cleared on reset either - a bug too?
Yes, I think we'll just have to scan the list of things that we want
zero'ed at start.
Not at start, at reset.
Similar case.
Amit
Re: [Qemu-devel] [PATCH] xen_disk: remove syncwrite option
On Tue, 24 Apr 2012, Kevin Wolf wrote:
Am 24.04.2012 13:22, schrieb Stefano Stabellini:
Use the BDRV_O_CACHE_* flags instead.
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Doesn't apply to qemu.git because...
---
hw/xen_disk.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 4a6d89c..3e4a47b 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -48,7 +48,6 @@
/* - */
-static int syncwrite= 0;
static int batch_maps = 0;
static int max_requests = 32;
@@ -189,15 +188,10 @@ static int ioreq_parse(struct ioreq *ioreq)
ioreq-presync = 1;
return 0;
}
-if (!syncwrite) {
-ioreq-postsync = 1;
...this is ioreq-presync = ioreq-postsync = 1;
That's because it is based on top of the following two patches:
xen_disk: use bdrv_aio_flush instead of bdrv_flush
http://marc.info/?l=qemu-develm=133434258008959w=2
xen_disk: implement BLKIF_OP_FLUSH_DISKCACHE, remove BLKIF_OP_WRITE_BARRIER
http://marc.info/?l=qemu-develm=133459581417878w=2
I have just sent a pull request for them
(http://marc.info/?l=qemu-develm=133526669019403) after leaving lying
around for a while.
Next time do you want me to send xen_disk specific pull request to you
instead? Sorry if I shouldn't have sent the pull request directly...
And while we're at it, the commit message could mention that there was
no way to set this flag anyway, so we're just removing dead code.
good point
Re: [Qemu-devel] [PATCH] xen_disk: remove syncwrite option
Am 24.04.2012 13:22, schrieb Stefano Stabellini:
Use the BDRV_O_CACHE_* flags instead.
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Doesn't apply to qemu.git because...
---
hw/xen_disk.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 4a6d89c..3e4a47b 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -48,7 +48,6 @@
/* - */
-static int syncwrite= 0;
static int batch_maps = 0;
static int max_requests = 32;
@@ -189,15 +188,10 @@ static int ioreq_parse(struct ioreq *ioreq)
ioreq-presync = 1;
return 0;
}
-if (!syncwrite) {
-ioreq-postsync = 1;
...this is ioreq-presync = ioreq-postsync = 1;
And while we're at it, the commit message could mention that there was
no way to set this flag anyway, so we're just removing dead code.
Kevin
Re: [Qemu-devel] [PATCH v3] spice_info: add mouse_mode
On 03/29/12 23:23, Alon Levy wrote: Add mouse_mode, either server or mouse, to qmp and hmp commands, based on spice_server_is_server_mouse added in spice-server 0.10.3. Patch added to spice patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
Am 24.04.2012 12:28, schrieb Pavel Hrdina:
On 04/24/2012 12:23 PM, Kevin Wolf wrote:
Am 24.04.2012 11:55, schrieb Pavel Hrdina:
On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive
controller emulation
when no media is present. If the guest is booted without a media then the
drive
was not being emulated at all but this patch enables the emulation with
no media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and
the behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior
like
bare metal with real floppy device (you have to load floppy driver at
first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the
virtual drive
when you want to open it but driver successfully detect that there is no
media in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with
-nodefaults and both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdinaphrd...@redhat.com
Signed-off-by: Michal Novotnyminov...@redhat.com
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs,nb_heads,max_track,
last_sect,
drv-drive,drive,rate);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
This modification is needed for floppy driver in guest to detect floppy
drive.
My question was more about how the floppy drivers in the guest detect
drives. They can't be relying on the geometry of a medium because no
medium is inserted. So setting the geometry must have some side effect
that I'm not aware of.
Well, this is good question, I'll investigate little bit more about this
and probably send new version of this patch.
Ok, thanks. I think it's important to understand _why_ a patch works,
not just _that_ it works (for a specific case).
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
And this is needed for all cases if there is no media in drive. Code in
fdctrl_change_cb() is needed only for detect that there is no media when
you try to mount it (linux guest) or open it (windows guest).
Hm. There seems to be more wrong that just this. I think the main
problem is that we clear the bit after reading it out once, whereas it
seems to stay set in reality. It would only be cleared once some command
(not sure which are possible) succeeds with a
Re: [Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
Am 23.04.2012 20:30, schrieb Marcelo Tosatti: On Mon, Apr 23, 2012 at 02:33:49PM +0200, Kevin Wolf wrote: Am 23.04.2012 01:35, schrieb Marcelo Tosatti: On Sun, Apr 22, 2012 at 08:18:49PM -0300, Marcelo Tosatti wrote: On Fri, Apr 20, 2012 at 03:56:01PM +0200, Kevin Wolf wrote: Refcount block allocation and refcount table growth rely on s-free_cluster_index pointing to somewhere after the current allocation. Change qcow2_allocate_cluster_at() to fulfill this assumption. Without this change it could happen that a newly allocated refcount block and the allocated data block point to the same area in the image file, causing data corruption in the long run. This fixes a bug that became first visible after commit 250196f1. Signed-off-by: Kevin Wolf kw...@redhat.com Kevin, This patch fixes explicit filesystem errors (qemu-img check also OK), but autotest is still failing, see attached screenshot. It is not reproducible without f081987ad20a8c8dc391deded55161ea8d38be5f Sorry, i meant commit 250196f19c6e7df12965d74a5073e10aba06c802 Author: Kevin Wolf kw...@redhat.com Date: Fri Mar 2 14:10:54 2012 +0100 qcow2: Reduce number of I/O requests The screenshot doesn't really give a lot of information, but let's assume that _something_ must have been corrupted... Can you try finding the corrupted file (e.g. using rpm -V) and see in which way it differs from the real one? Kevin Unfortunately no, /boot/vmlinuz is gone on the last install (screenshot). It should be easy to reproduce, install Fedora.8.64, or i can upload an image later tonight if that is helpful. Nope, can't reproduce. Fedora 8 installs and boots just fine (8 GB disk, default qcow2 cluster size, just clicked through the installer with default options). Kevin
Re: [Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()
On 04/24/2012 03:01 AM, Paolo Bonzini wrote: Il 24/04/2012 10:49, Stefan Hajnoczi ha scritto: The error is specific to block job speeds so we can add a speed argument to block-stream in the future and clearly identify the invalid parameter. I added the new error to avoid having to change the InvalidParameter 'name' field. It becomes ugly because we have: block-job-set-speed device value block-stream device [speed] [base] Notice that the parameter is called 'value' in block-job-set-speed and 'speed' in block-stream. Therefore we can't just propagate a single InvalidParameter name='value' error up from block-stream. This means that QMP commands will need to change the error to use the correct name :(. It's doable, but ugly and a bit more complex. Well, we still have time to change block-job-set-speed. Ugly, but doable. block-job-set-speed device speed would indeed let you use 'speed' as the parameter name in both calls, and I can just as easily make libvirt target the name 'value' for block_job_set_speed and 'speed' for block-job-set-speed. I'm okay with that idea, if it makes the rest of your patch easier. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media
On 04/24/2012 12:23 PM, Kevin Wolf wrote:
Am 24.04.2012 11:55, schrieb Pavel Hrdina:
On 04/24/2012 11:32 AM, Kevin Wolf wrote:
Am 23.04.2012 18:06, schrieb Pavel Hrdina:
Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no
media present.
There was a bug in FDC emulation without media. Driver was not able to
recognize that
there is no media in drive.
This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the
behaviour
is as expected, i.e. as follows:
Linux guest (Fedora 16 x86_64) tries mount /dev/fd0 and exit with error
mount: /dev/fd0 is not a valid block device which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. modprobe floppy command).
For Windows XP guest the Windows floppy driver is trying to seek the virtual
drive
when you want to open it but driver successfully detect that there is no media
in drive
and then it's asking user to insert floppy media in the drive.
I also tested behavior of this patch if you start guest with -nodefaults and
both
Windows and Linux guests detect only FDC but no drive.
Pavel
This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller
Signed-off-by: Pavel Hrdinaphrd...@redhat.com
Signed-off-by: Michal Novotnyminov...@redhat.com
It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.
---
hw/fdc.c | 14 ++
hw/pc.c |3 ++-
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
FDriveRate rate;
FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL bdrv_is_inserted(drv-bs)) {
+if (drv-bs != NULL) {
ro = bdrv_is_read_only(drv-bs);
bdrv_get_floppy_geometry_hint(drv-bs,nb_heads,max_track,
last_sect, drv-drive,drive,rate);
I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5 1.44 MB floppies, or more precisely:
{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?
ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.
This modification is needed for floppy driver in guest to detect floppy drive.
My question was more about how the floppy drivers in the guest detect
drives. They can't be relying on the geometry of a medium because no
medium is inserted. So setting the geometry must have some side effect
that I'm not aware of.
Well, this is good question, I'll investigate little bit more about this
and probably send new version of this patch.
//
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
if (!drv-bs)
return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv-bs))
+return 1;
In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?
Or do you in fact need it for the initial state?
As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
, for specification of DIR register. Bit7 is there as CHAN and in this
bit is saved information whether media is changed or not. This bit is
set to true while there is no media. And floppy driver is checking this
bit to detect media change or media missing.
And this is needed for all cases if there is no media in drive. Code in
fdctrl_change_cb() is needed only for detect that there is no media when
you try to mount it (linux guest) or open it (windows guest).
Hm. There seems to be more wrong that just this. I think the main
problem is that we clear the bit after reading it out once, whereas it
seems to stay set in reality. It would only be cleared once some command
(not sure which are possible) succeeds with a newly inserted disk.
Unfortunately I couldn't really find any appropriate documentation for
the bit. The FDC spec says DSKCHG monitors the pin of the same name and
reflects the opposite value seen on the disk cable, but I couldn't find
any description
[Qemu-devel] [PATCH] usb-uhci: update irq line on reset
uhci_reset() clears irq mask and irq status registers, but doesn't update the irq line. Which may result in suspious IRQs after uhci reset. Fix it. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/hcd-uhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 266d550..9e211a0 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -369,6 +369,7 @@ static void uhci_reset(void *opaque) } uhci_async_cancel_all(s); +uhci_update_irq(s); } static void uhci_pre_save(void *opaque) -- 1.7.1
Re: [Qemu-devel] [PATCH] xen_disk: remove syncwrite option
On Tue, 24 Apr 2012, Kevin Wolf wrote: That's because it is based on top of the following two patches: xen_disk: use bdrv_aio_flush instead of bdrv_flush http://marc.info/?l=qemu-develm=133434258008959w=2 xen_disk: implement BLKIF_OP_FLUSH_DISKCACHE, remove BLKIF_OP_WRITE_BARRIER http://marc.info/?l=qemu-develm=133459581417878w=2 I have just sent a pull request for them (http://marc.info/?l=qemu-develm=133526669019403) after leaving lying around for a while. Next time do you want me to send xen_disk specific pull request to you instead? Sorry if I shouldn't have sent the pull request directly... I see. I wasn't aware that you're doing pull requests, but it isn't a problem. It just means that you should probably do a pull request for this one as well instead of expecting that I pick it up. But you can have my Acked-by, if you like. Should I ignore xen_disk patches from now on for the block branch and assume that you pick them up? I am happy to issue pull requests for xen_disk patches, but I still welcome your reviews :-)
Re: [Qemu-devel] [PATCH] qxl: don't assert on guest create_guest_primary
On Tue, Apr 24, 2012 at 12:17:15PM +0200, Gerd Hoffmann wrote: On 04/18/12 13:00, Alon Levy wrote: initiate the implicit destroy ourselves. What is the use case? Don't let guest crash the machine with bad driver. i.e. during testing I did this inadvertently. Allowing guests to do that without giving them a way to figure whenever it will work or not doesn't look that useful to me. OK - so maybe just have this as a guest_bug instead of an assert? cheers, Gerd
Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init
On Tue, Apr 24, 2012 at 01:23:53PM +0200, Andreas Färber wrote: Am 24.04.2012 12:26, schrieb Amit Shah: On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote: On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote: From: Alon Levy al...@redhat.com guest_connected should be false before guest driver initialization, and true after, both for multiport aware and non multiport aware drivers. Don't set it before the guest_features are available; instead use set_status which is called by io to VIRTIO_PCI_STATUS with VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers. [Amit: Add comment, tweak summary] Logic also changed fron Alon's patch. Why? Yes, forgot to mention that here. Re here: FWIW I learnt to add these sort of comments before the new SoB, i.e. between mst's Acked-by and your Signed-off-by so that it is clear that he ack'ed the previous version given the logic change. In this case it's arguably irrelevant since there's only one SoB by you. Good idea. Note about the original patch - I didn't notice the side effect I didn't put in the commit log, and then I missed Amit's change of my code as well :) Andreas Signed-off-by: Alon Levy al...@redhat.com Acked-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Amit Shah amit.s...@redhat.com -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] copy benchmarks onto qemu
i got it working with libguestfs. libguestfs is awesome at manipulating guest VM images. Xin On Tue, Apr 24, 2012 at 4:45 AM, Mulyadi Santosa mulyadi.sant...@gmail.com wrote: On Tue, Apr 24, 2012 at 02:09, Xin Tong xerox.time.t...@gmail.com wrote: I am not too sure what you mean by raw image. what i have is an *.img file that is bootable by QEMU. will kpartx work ? try to use qemu-img info to find what format the file uses... -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com
[Qemu-devel] Assertion after chaning display resolution
Hi all, I saw the following assert after chaning display resolution. This might be the cause, but i am not sure. Threaded VNC is enabled. Anyone ever seen this? qemu-kvm-1.0: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) ((av)-bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd old_size == 0) || ((unsigned long) (old_size) = (unsigned long)__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) ~((2 * (sizeof(size_t))) - 1))) ((old_top)-size 0x1) ((unsigned long)old_end pagemask) == 0)' failed. Thanks, Peter
[Qemu-devel] [PATCH v2 0/2] Updated fixes for block job cancellation
These patches fix the existing problems with synchronous job cancellation without adding a bdrv_drain_all in the asynchronous block_job_cancel. Paolo Bonzini (2): block: add block_job_sleep_ns block: wait for job callback in block_job_cancel_sync block.c| 47 +-- block/stream.c | 29 +++-- block_int.h| 33 + 3 files changed, 81 insertions(+), 28 deletions(-) -- 1.7.9.3
[Qemu-devel] [PATCH v2 2/2] block: wait for job callback in block_job_cancel_sync
The limitation on not having I/O after cancellation cannot really be
honored. Even streaming has a very small race window where you could
cancel a job and have it report completion. If this window is hit,
bdrv_change_backing_file() will yield and possibly cause accesses to
dangling pointers etc.
So, let's just assume that we cannot know exactly what will happen
after the coroutine has set busy to false. We can set a very lax
condition:
- if we cancel the job, the coroutine won't set it to false again
(and hence will not call co_sleep_ns again).
- block_job_cancel_sync will wait for the coroutine to exit, which
pretty much ensures no race.
Instead, we track the coroutine that executes the job and put very
strict conditions on what to do while it is quiescent (busy = false).
First of all, the coroutine must never set busy = false while the job
has been cancelled. Second, the coroutine can be reentered arbitrarily
while it is quiescent, so you cannot really do anything but co_sleep_ns at
that time. This condition is obeyed by the block_job_sleep_ns function.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
block.c| 36 ++--
block/stream.c |7 +++
block_int.h| 11 ++-
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 60820bf..b0176c4 100644
--- a/block.c
+++ b/block.c
@@ -4228,6 +4228,9 @@ int block_job_set_speed(BlockJob *job, int64_t value)
void block_job_cancel(BlockJob *job)
{
job-cancelled = true;
+if (job-co !job-busy) {
+qemu_coroutine_enter(job-co, NULL);
+}
}
bool block_job_is_cancelled(BlockJob *job)
@@ -4235,15 +4238,44 @@ bool block_job_is_cancelled(BlockJob *job)
return job-cancelled;
}
-void block_job_cancel_sync(BlockJob *job)
+struct BlockCancelData {
+BlockJob *job;
+BlockDriverCompletionFunc *cb;
+void *opaque;
+bool cancelled;
+int ret;
+};
+
+static void block_job_cancel_cb(void *opaque, int ret)
{
+struct BlockCancelData *data = opaque;
+
+data-cancelled = block_job_is_cancelled(data-job);
+data-ret = ret;
+data-cb(data-opaque, ret);
+}
+
+int block_job_cancel_sync(BlockJob *job)
+{
+struct BlockCancelData data;
BlockDriverState *bs = job-bs;
assert(bs-job == job);
+
+/* Set up our own callback to store the result and chain to
+ * the original callback.
+ */
+data.job = job;
+data.cb = job-cb;
+data.opaque = job-opaque;
+data.ret = -EINPROGRESS;
+job-cb = block_job_cancel_cb;
+job-opaque = data;
block_job_cancel(job);
-while (bs-job != NULL bs-job-busy) {
+while (data.ret == -EINPROGRESS) {
qemu_aio_wait();
}
+return (data.cancelled data.ret == 0) ? -ECANCELED : data.ret;
}
void block_job_sleep_ns(BlockJob *job, QEMUClock *clock, int64_t ns)
diff --git a/block/stream.c b/block/stream.c
index a2bb854..de999af 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -185,7 +185,6 @@ int stream_start(BlockDriverState *bs, BlockDriverState
*base,
void *opaque)
{
StreamBlockJob *s;
-Coroutine *co;
s = block_job_create(stream_job_type, bs, cb, opaque);
if (!s) {
@@ -197,8 +196,8 @@ int stream_start(BlockDriverState *bs, BlockDriverState
*base,
pstrcpy(s-backing_file_id, sizeof(s-backing_file_id), base_id);
}
-co = qemu_coroutine_create(stream_run);
-trace_stream_start(bs, base, s, co, opaque);
-qemu_coroutine_enter(co, s);
+s-common.co = qemu_coroutine_create(stream_run);
+trace_stream_start(bs, base, s, s-common.co, opaque);
+qemu_coroutine_enter(s-common.co, s);
return 0;
}
diff --git a/block_int.h b/block_int.h
index c1f8649..eda021d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -95,6 +95,12 @@ struct BlockJob {
BlockDriverState *bs;
/**
+ * The coroutine that executes the job. If not NULL, it is
+ * reentered when busy is false and the job is cancelled.
+ */
+Coroutine *co;
+
+/**
* Set to true if the job should cancel itself. The flag must
* always be tested just before toggling the busy flag from false
* to true. After a job has been cancelled, it should only yield
@@ -414,8 +420,11 @@ bool block_job_is_cancelled(BlockJob *job);
* immediately after #block_job_cancel_sync. Users of block jobs
* will usually protect the BlockDriverState objects with a reference
* count, should this be a concern.
+ *
+ * Returns the return value from the job if the job actually completed
+ * during the call, or -ECANCELED if it was canceled.
*/
-void block_job_cancel_sync(BlockJob *job);
+int block_job_cancel_sync(BlockJob *job);
/**
* stream_start:
--
1.7.9.3
