[Qemu-devel] [PATCH] eepro100: Better documentation for temporary data
Michael S. Tsirkin suggested more documentation for
two temporary status values. Well, here it is.
Signed-off-by: Stefan Weil
---
hw/eepro100.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 22e5bed..5a77409 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -213,9 +213,10 @@ typedef struct {
uint32_t ru_offset; /* RU address offset */
uint32_t statsaddr; /* pointer to eepro100_stats_t */
-/* Temporary data. */
-eepro100_tx_t tx;
-uint32_t cb_address;
+/* Temporary status information (no need to save these values),
+ * used while processing CU commands. */
+eepro100_tx_t tx; /* transmit buffer descriptor */
+uint32_t cb_address;/* = cu_base + cu_offset */
/* Statistical counters. Also used for wake-up packet (i82559). */
eepro100_stats_t statistics;
--
1.6.5
[Qemu-devel] [PATCH] s390: Fix buggy assignment
nd->model keeps dynamically allocated model names.
So casting of a constant string is wrong here.
Signed-off-by: Stefan Weil
---
hw/s390-virtio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index b567886..b57fa9c 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -209,7 +209,7 @@ static void s390_init(ram_addr_t ram_size,
DeviceState *dev;
if (!nd->model) {
-nd->model = (char*)"virtio";
+nd->model = qemu_strdup("virtio");
}
if (strcmp(nd->model, "virtio")) {
--
1.6.5
[FOR 0.12 PATCH] qdev: Improve uni-north device names (was: [Qemu-devel] [FOR 0.12 PATCH] qdev: Replace device names containing whitespace)
Switch to the names suggested by Blue Swirl.
Signed-off-by: Markus Armbruster
---
hw/unin_pci.c | 24
1 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index f07c966..3ae4e7a 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -148,7 +148,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
/* Use values found on a real PowerMac */
/* Uninorth main bus */
-dev = qdev_create(NULL, "uni-north-main");
+dev = qdev_create(NULL, "uni-north");
qdev_init_nofail(dev);
s = sysbus_from_qdev(dev);
d = FROM_SYSBUS(UNINState, s);
@@ -157,7 +157,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
pic, 11 << 3, 4);
#if 0
-pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-main");
+pci_create_simple(d->host_state.bus, 11 << 3, "uni-north");
#endif
sysbus_mmio_map(s, 0, 0xf280);
@@ -170,8 +170,8 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
#endif
/* Uninorth AGP bus */
-pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-AGP");
-dev = qdev_create(NULL, "uni-north-AGP");
+pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-agp");
+dev = qdev_create(NULL, "uni-north-agp");
qdev_init_nofail(dev);
s = sysbus_from_qdev(dev);
sysbus_mmio_map(s, 0, 0xf080);
@@ -180,8 +180,8 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
/* Uninorth internal bus */
#if 0
/* XXX: not needed for now */
-pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-internal");
-dev = qdev_create(NULL, "uni-north-internal");
+pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-pci");
+dev = qdev_create(NULL, "uni-north-pci");
qdev_init_nofail(dev);
s = sysbus_from_qdev(dev);
sysbus_mmio_map(s, 0, 0xf480);
@@ -260,7 +260,7 @@ static int unin_internal_pci_host_init(PCIDevice *d)
}
static PCIDeviceInfo unin_main_pci_host_info = {
-.qdev.name = "uni-north-main",
+.qdev.name = "uni-north",
.qdev.size = sizeof(PCIDevice),
.init = unin_main_pci_host_init,
};
@@ -272,29 +272,29 @@ static PCIDeviceInfo dec_21154_pci_host_info = {
};
static PCIDeviceInfo unin_agp_pci_host_info = {
-.qdev.name = "uni-north-AGP",
+.qdev.name = "uni-north-agp",
.qdev.size = sizeof(PCIDevice),
.init = unin_agp_pci_host_init,
};
static PCIDeviceInfo unin_internal_pci_host_info = {
-.qdev.name = "uni-north-internal",
+.qdev.name = "uni-north-pci",
.qdev.size = sizeof(PCIDevice),
.init = unin_internal_pci_host_init,
};
static void unin_register_devices(void)
{
-sysbus_register_dev("uni-north-main", sizeof(UNINState),
+sysbus_register_dev("uni-north", sizeof(UNINState),
pci_unin_main_init_device);
pci_qdev_register(&unin_main_pci_host_info);
sysbus_register_dev("dec-21154", sizeof(UNINState),
pci_dec_21154_init_device);
pci_qdev_register(&dec_21154_pci_host_info);
-sysbus_register_dev("uni-north-AGP", sizeof(UNINState),
+sysbus_register_dev("uni-north-agp", sizeof(UNINState),
pci_unin_agp_init_device);
pci_qdev_register(&unin_agp_pci_host_info);
-sysbus_register_dev("uni-north-internal", sizeof(UNINState),
+sysbus_register_dev("uni-north-pci", sizeof(UNINState),
pci_unin_internal_init_device);
pci_qdev_register(&unin_internal_pci_host_info);
}
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/13/09 21:43, Michael S. Tsirkin wrote: Add features property to virtio. This makes it possible to e.g. define machine without indirect buffer support, which is required for 0.10 compatibility. or without hardware checksum support, which is required for 0.11 compatibility. I'd suggest to add flags for the individual features to the drivers which actually use it instead, so you'll have -device virtio-net-pci,hw-checksum=0 and -device virtio-blk-pci,indirect-buffers=0 cheers, Gerd
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On Mon, Dec 14, 2009 at 10:41:26AM +0100, Gerd Hoffmann wrote: > On 12/13/09 21:43, Michael S. Tsirkin wrote: >> Add features property to virtio. This makes it >> possible to e.g. define machine without indirect >> buffer support, which is required for 0.10 >> compatibility. or without hardware checksum >> support, which is required for 0.11 compatibility. > > I'd suggest to add flags for the individual features to the drivers > which actually use it instead, so you'll have > > -device virtio-net-pci,hw-checksum=0 > > and > > -device virtio-blk-pci,indirect-buffers=0 > > cheers, > Gerd Hmm. I hoped to avoid it, there are lots of features so it's a lot of work and in practice, this will most likely be set by machine description ... -- MST
[Qemu-devel] [FOR 0.12 PATCH] Simplify qemu_realloc()
No functional change. Bonus: looks just like qemu_malloc() now.
Signed-off-by: Markus Armbruster
---
I tagged this "FOR 0.12" because I think you might want to consider it
for 0.12, not because I think it must go into 0.12.
qemu-malloc.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 5d9e34d..6cdc5de 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -61,12 +61,10 @@ void *qemu_malloc(size_t size)
void *qemu_realloc(void *ptr, size_t size)
{
-if (size) {
-return oom_check(realloc(ptr, size));
-} else if (allow_zero_malloc()) {
-return oom_check(realloc(ptr, size ? size : 1));
+if (!size && !allow_zero_malloc()) {
+abort();
}
-abort();
+return oom_check(realloc(ptr, size ? size : 1));
}
void *qemu_mallocz(size_t size)
[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
> >>> From d9dc0f50b2ce756e8a3b4ede0a8ecbe76f2afcb8 Mon Sep 17 00:00:00 2001
> >>From: Magnus Christensson
> >>Date: Wed, 25 Nov 2009 16:26:58 +0100
> >>Subject: [PATCH 13/13] Fix PkgLength calculation for the SSDT.
> >>
> >>Signed-off-by: Magnus Christensson
> >>---
> >> src/acpi.c |6 --
> >> 1 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/src/acpi.c b/src/acpi.c
> >>index 843af69..88007ae 100644
> >>--- a/src/acpi.c
> >>+++ b/src/acpi.c
> >>@@ -464,10 +464,12 @@ build_ssdt(void)
> >> // build processor scope header
> >> *(ssdt_ptr++) = 0x10; // ScopeOp
> >> if (cpu_length<= 0x3e) {
> >>+/* Handle 1-4 CPUs with one byte encoding */
> >> *(ssdt_ptr++) = cpu_length + 1;
> >> } else {
> >>-*(ssdt_ptr++) = 0x7F;
> >>-*(ssdt_ptr++) = (cpu_length + 2)>> 6;
> >>+/* Handle 5-314 CPUs with two byte encoding */
> >>+*(ssdt_ptr++) = 0x40 | ((cpu_length + 1)& 0xf);
> >>+*(ssdt_ptr++) = (cpu_length + 1)>> 4;
> >Should be cpu_length + 2 as far as I can tell. The current code is
> >definitely broken.
> Right. That should be cpu_length +2 in the else-part.
>
BTW, how did you notice it? What OS fails?
--
Gleb.
[Qemu-devel] [PATCH] target-alpha: Fix compiler warning for gcc-4.3 (and older)
"Old" compilers obviously are not able to recognise that all cases are handled here: qemu/target-alpha/helper.c:70: error: ‘round_mode’ may be used uninitialized in this function A small modification helps the compiler to do its jobs. gcc-4.4 does not need this, but is still not standard on all platforms. Signed-off-by: Stefan Weil --- target-alpha/helper.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/target-alpha/helper.c b/target-alpha/helper.c index a658f97..be7d37b 100644 --- a/target-alpha/helper.c +++ b/target-alpha/helper.c @@ -95,6 +95,7 @@ void cpu_alpha_store_fpcr (CPUState *env, uint64_t val) round_mode = float_round_nearest_even; break; case 3: +default: /* this avoids a gcc (< 4.4) warning */ round_mode = float_round_up; break; } -- 1.6.5
[Qemu-devel] Re: [PATCH] s390: Fix buggy assignment
On 14.12.2009, at 10:39, Stefan Weil wrote: > nd->model keeps dynamically allocated model names. > So casting of a constant string is wrong here. > > Signed-off-by: Stefan Weil So we're wasting 7 bytes of strdup malloc'ed data we never free. Probably now worth thinking about though. Acked-by: Alexander Graf
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/14/09 10:42, Michael S. Tsirkin wrote: On Mon, Dec 14, 2009 at 10:41:26AM +0100, Gerd Hoffmann wrote: On 12/13/09 21:43, Michael S. Tsirkin wrote: Add features property to virtio. This makes it possible to e.g. define machine without indirect buffer support, which is required for 0.10 compatibility. or without hardware checksum support, which is required for 0.11 compatibility. I'd suggest to add flags for the individual features to the drivers which actually use it instead, so you'll have -device virtio-net-pci,hw-checksum=0 and -device virtio-blk-pci,indirect-buffers=0 cheers, Gerd Hmm. I hoped to avoid it, there are lots of features so it's a lot of work and in practice, this will most likely be set by machine description ... MSI-X aka vectors property is already done this way, so I'd tend to continue this way. It is also more user friendly. Sure, these are most likely not used on a daily base by users, but being able to turn off -- say -- indirect buffers for testing and/or bug hunting reasons without having to construct magic hex numbers from virtio header files would be nice. Can you give a list of features? The patch description sounded like it is just the two listed above ... cheers, Gerd
[Qemu-devel] Re: -serial stdio broken
On 12/13/09 10:38, Blue Swirl wrote: On Sun, Dec 13, 2009 at 8:24 AM, Blue Swirl wrote: I guess e1c09175bc00dd8dfb2ad1b26e1858dcdc109b59 or 998bbd74b9d813b14a3a3b5009a5d5a48c7dce51 broke -serial stdio for all targets: qemu -serial stdio -monitor stdio Oh. It is actually used on the command line. Hmm. First, you can use '-serial mon:stdio' instead. Second, with 'qemu -nographic' you don't need to specify this at all because that is the default. What is gone now is the automagic conversion of '-serial stdio -monitor stdio' into '-serial mon:stdio' because I didn't expect people actually using that on the command line (see commit message). Also this kind of post-processing is pretty horrible thing for the command line parser code. Thus I would pretty much prefer to not re-introduce this ... Can you live with one of the alternatives outlined above? cheers, Gerd
[Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > On 12/14/09 10:44, Michael S. Tsirkin wrote: >> No, it did not even start booting the kernel. Just gave me blank screen. > > [ testing ] > > Oh. That is something completely different. A bug in the rom loader. > It fails to fit both e1000 (default nic) and virtio-net boot roms into > the option rom area and bails out (before loading seabios). vl.c > doesn't check the return value and happily continues (without bios). > Which doesn't work out very well ... > > With two identical nics the (single) rom fits and qemu boots. > > Hmm. Of course vl.c must be fixed to check the return value. Yes. > Not sure how to deal with the rom size issue. The gPXE roms look quit > big compared to the older roms we had. Hmm, it's a regression then ... > Do they have tons of features > compiled in? Can we make them smaller by disabling some? Or maybe have > *one* rom which supports *all* qemu nics? It would be larger of course, > but assuming a big share of the option rom is non-hardware specific code > which wouldn't be duplicated in memory then with two different cards it > might work out nevertheless? Haven't looked at the code at all yet. I would expect common code with all the features to be part of PXE code in BIOS, not part of option rom. Isn't this how it is structured? > Or add a nic property to disable rom > loading? Yes, and I think it should be off by default. > cheers, > Gerd Cc qemu and other people who participated in a similar discussion in the past. -- MST
[Qemu-devel] [PATCH] Check rom_load_all() return value.
... otherwise we'll continue without the bios loaded in case the
option roms don't fit.
Signed-off-by: Gerd Hoffmann
---
vl.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index c0d98f5..1682808 100644
--- a/vl.c
+++ b/vl.c
@@ -6031,7 +6031,10 @@ int main(int argc, char **argv, char **envp)
qdev_machine_creation_done();
-rom_load_all();
+if (rom_load_all() != 0) {
+fprintf(stderr, "rom loading failed\n");
+exit(1);
+}
qemu_system_reset();
if (loadvm) {
--
1.6.5.2
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
> On 12/14/09 10:42, Michael S. Tsirkin wrote:
>> On Mon, Dec 14, 2009 at 10:41:26AM +0100, Gerd Hoffmann wrote:
>>> On 12/13/09 21:43, Michael S. Tsirkin wrote:
Add features property to virtio. This makes it
possible to e.g. define machine without indirect
buffer support, which is required for 0.10
compatibility. or without hardware checksum
support, which is required for 0.11 compatibility.
>>>
>>> I'd suggest to add flags for the individual features to the drivers
>>> which actually use it instead, so you'll have
>>>
>>>-device virtio-net-pci,hw-checksum=0
>>>
>>> and
>>>
>>>-device virtio-blk-pci,indirect-buffers=0
>>>
>>> cheers,
>>>Gerd
>>
>> Hmm. I hoped to avoid it, there are lots of features so it's a lot of
>> work and in practice, this will most likely be set by machine
>> description ...
>
> MSI-X aka vectors property is already done this way, so I'd tend to
> continue this way. It is also more user friendly. Sure, these are most
> likely not used on a daily base by users, but being able to turn off --
> say -- indirect buffers for testing and/or bug hunting reasons without
> having to construct magic hex numbers from virtio header files would be
> nice.
>
> Can you give a list of features? The patch description sounded like it
> is just the two listed above ...
>
> cheers,
> Gerd
Heh, it's a long list.
transport features (common to all):
VIRTIO_F_NOTIFY_ON_EMPTY;
VIRTIO_RING_F_INDIRECT_DESC;
VIRTIO_F_BAD_FEATURE; <- not sure we need a flag for this
for net:
uint32_t features = (1 << VIRTIO_NET_F_MAC) |
(1 << VIRTIO_NET_F_MRG_RXBUF) |
(1 << VIRTIO_NET_F_STATUS) |
(1 << VIRTIO_NET_F_CTRL_VQ) |
(1 << VIRTIO_NET_F_CTRL_RX) |
(1 << VIRTIO_NET_F_CTRL_VLAN) |
(1 << VIRTIO_NET_F_CTRL_RX_EXTRA);
if (peer_has_vnet_hdr(n)) {
tap_using_vnet_hdr(n->nic->nc.peer, 1);
features |= (1 << VIRTIO_NET_F_CSUM);
features |= (1 << VIRTIO_NET_F_HOST_TSO4);
features |= (1 << VIRTIO_NET_F_HOST_TSO6);
features |= (1 << VIRTIO_NET_F_HOST_ECN);
features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
features |= (1 << VIRTIO_NET_F_GUEST_ECN);
if (peer_has_ufo(n)) {
features |= (1 << VIRTIO_NET_F_GUEST_UFO);
features |= (1 << VIRTIO_NET_F_HOST_UFO);
}
for block:
features |= (1 << VIRTIO_BLK_F_SEG_MAX);
features |= (1 << VIRTIO_BLK_F_GEOMETRY);
if (bdrv_enable_write_cache(s->bs))
features |= (1 << VIRTIO_BLK_F_WCACHE);
#ifdef __linux__
features |= (1 << VIRTIO_BLK_F_SCSI);
#endif
if (strcmp(s->serial_str, "0"))
features |= 1 << VIRTIO_BLK_F_IDENTIFY;
if (bdrv_is_read_only(s->bs))
features |= 1 << VIRTIO_BLK_F_RO;
I could try and group features, but this way we
loose in flexibility ...
How about I name properties exactly like virtio macros? e.g.
VIRTIO_BLK_F_IDENTIFY etc? This way maybe I can use preprocessor magic
to reduce duplication ...
Also, I'd like these things to be saved in bits and not add a ton
of fields in device. Ideas how to do this?
--
MST
[Qemu-devel] [PATCH] correcting ARM CPSR register bit position comment
From: nemesis
---
target-arm/cpu.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 4a1c53f..910604f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -90,7 +90,7 @@ typedef struct CPUARMState {
/* cpsr flag cache for faster execution */
uint32_t CF; /* 0 or 1 */
-uint32_t VF; /* V is the bit 31. All other bits are undefined */
+uint32_t VF; /* V is the bit 28. */
uint32_t NF; /* N is bit 31. All other bits are undefined. */
uint32_t ZF; /* Z set if zero. */
uint32_t QF; /* 0 or 1 */
--
1.6.3.3
[Qemu-devel] [FOR 0.12][RESEND][PATCH] kvm: x86: Use separate exception_injected CPUState field
Marcelo correctly remarked that there are usage conflicts between QEMU
core code and KVM /wrt exception_index. So spend a separate field and
also save/restore it properly.
Signed-off-by: Jan Kiszka
---
NOTE: This obsoletes the meanwhile merge patch
4d6e3ac5d411c461d0fb4b1cd2ace854963c9e30, please revert it!
target-i386/cpu.h |1 +
target-i386/kvm.c |6 +++---
target-i386/machine.c |1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9ef1be4..afb4da5 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -694,6 +694,7 @@ typedef struct CPUX86State {
/* For KVM */
uint32_t mp_state;
+int32_t exception_injected;
int32_t interrupt_injected;
uint8_t soft_interrupt;
uint8_t nmi_injected;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 53955b4..de79eb7 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -720,8 +720,8 @@ static int kvm_put_vcpu_events(CPUState *env)
return 0;
}
-events.exception.injected = (env->exception_index >= 0);
-events.exception.nr = env->exception_index;
+events.exception.injected = (env->exception_injected >= 0);
+events.exception.nr = env->exception_injected;
events.exception.has_error_code = env->has_error_code;
events.exception.error_code = env->error_code;
@@ -755,7 +755,7 @@ static int kvm_get_vcpu_events(CPUState *env)
if (ret < 0) {
return ret;
}
-env->exception_index =
+env->exception_injected =
events.exception.injected ? events.exception.nr : -1;
env->has_error_code = events.exception.has_error_code;
env->error_code = events.exception.error_code;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 2fb8fab..567e01e 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -448,6 +448,7 @@ static const VMStateDescription vmstate_cpu = {
VMSTATE_INT32_V(interrupt_injected, CPUState, 9),
VMSTATE_UINT32_V(mp_state, CPUState, 9),
VMSTATE_UINT64_V(tsc, CPUState, 9),
+VMSTATE_INT32_V(exception_injected, CPUState, 11),
VMSTATE_UINT8_V(soft_interrupt, CPUState, 11),
VMSTATE_UINT8_V(nmi_injected, CPUState, 11),
VMSTATE_UINT8_V(nmi_pending, CPUState, 11),
[Qemu-devel] [FOR 0.12][PATCH] target-i386: Fix evaluation of DR7 register
hw_breakpoint_type and hw_breakpoint_len used the wrong index multiplier
to extract type and len.
Signed-off-by: Jan Kiszka
---
target-i386/cpu.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9ef1be4..e835f23 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -834,12 +834,12 @@ static inline int hw_breakpoint_enabled(unsigned long
dr7, int index)
static inline int hw_breakpoint_type(unsigned long dr7, int index)
{
-return (dr7 >> (DR7_TYPE_SHIFT + (index * 2))) & 3;
+return (dr7 >> (DR7_TYPE_SHIFT + (index * 4))) & 3;
}
static inline int hw_breakpoint_len(unsigned long dr7, int index)
{
-int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 2))) & 3);
+int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
return (len == 2) ? 8 : len + 1;
}
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/14/09 12:10, Michael S. Tsirkin wrote:
On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
for block:
if (strcmp(s->serial_str, "0"))
features |= 1<< VIRTIO_BLK_F_IDENTIFY;
if (bdrv_is_read_only(s->bs))
features |= 1<< VIRTIO_BLK_F_RO;
Sure you want these be configurable?
Also, I'd like these things to be saved in bits and not add a ton
of fields in device. Ideas how to do this?
I guess you only want disable features?
You could have a bitmap property then, which accepts names for the bits.
It would need a table like this ...
char *bitnames[] = {
[ VIRTIO_BLK_F_IDENTIFY ] = "blk-identify",
[ VIRTIO_BLK_F_RO [ = "blk-ro",
[ ... ]
};
Then the property parser would accepts strings such as 'bit1|bit2' and
you can have
-device 'virtio-blk-pci,disable=blk-identify|ring-indirect'
The driver will just do 'vdev->host_features &= ~disable'.
cheers,
Gerd
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
Am 14.12.2009 um 12:10 schrieb "Michael S. Tsirkin" :
On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
On 12/14/09 10:42, Michael S. Tsirkin wrote:
On Mon, Dec 14, 2009 at 10:41:26AM +0100, Gerd Hoffmann wrote:
On 12/13/09 21:43, Michael S. Tsirkin wrote:
Add features property to virtio. This makes it
possible to e.g. define machine without indirect
buffer support, which is required for 0.10
compatibility. or without hardware checksum
support, which is required for 0.11 compatibility.
I'd suggest to add flags for the individual features to the drivers
which actually use it instead, so you'll have
-device virtio-net-pci,hw-checksum=0
and
-device virtio-blk-pci,indirect-buffers=0
cheers,
Gerd
Hmm. I hoped to avoid it, there are lots of features so it's a lot
of
work and in practice, this will most likely be set by machine
description ...
MSI-X aka vectors property is already done this way, so I'd tend to
continue this way. It is also more user friendly. Sure, these are
most
likely not used on a daily base by users, but being able to turn
off --
say -- indirect buffers for testing and/or bug hunting reasons
without
having to construct magic hex numbers from virtio header files
would be
nice.
Can you give a list of features? The patch description sounded
like it
is just the two listed above ...
cheers,
Gerd
Heh, it's a long list.
transport features (common to all):
VIRTIO_F_NOTIFY_ON_EMPTY;
VIRTIO_RING_F_INDIRECT_DESC;
VIRTIO_F_BAD_FEATURE; <- not sure we need a flag for this
for net:
uint32_t features = (1 << VIRTIO_NET_F_MAC) |
(1 << VIRTIO_NET_F_MRG_RXBUF) |
(1 << VIRTIO_NET_F_STATUS) |
(1 << VIRTIO_NET_F_CTRL_VQ) |
(1 << VIRTIO_NET_F_CTRL_RX) |
(1 << VIRTIO_NET_F_CTRL_VLAN) |
(1 << VIRTIO_NET_F_CTRL_RX_EXTRA);
if (peer_has_vnet_hdr(n)) {
tap_using_vnet_hdr(n->nic->nc.peer, 1);
features |= (1 << VIRTIO_NET_F_CSUM);
features |= (1 << VIRTIO_NET_F_HOST_TSO4);
features |= (1 << VIRTIO_NET_F_HOST_TSO6);
features |= (1 << VIRTIO_NET_F_HOST_ECN);
features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
features |= (1 << VIRTIO_NET_F_GUEST_ECN);
if (peer_has_ufo(n)) {
features |= (1 << VIRTIO_NET_F_GUEST_UFO);
features |= (1 << VIRTIO_NET_F_HOST_UFO);
}
for block:
features |= (1 << VIRTIO_BLK_F_SEG_MAX);
features |= (1 << VIRTIO_BLK_F_GEOMETRY);
if (bdrv_enable_write_cache(s->bs))
features |= (1 << VIRTIO_BLK_F_WCACHE);
#ifdef __linux__
features |= (1 << VIRTIO_BLK_F_SCSI);
#endif
if (strcmp(s->serial_str, "0"))
features |= 1 << VIRTIO_BLK_F_IDENTIFY;
if (bdrv_is_read_only(s->bs))
features |= 1 << VIRTIO_BLK_F_RO;
I could try and group features, but this way we
loose in flexibility ...
How about I name properties exactly like virtio macros? e.g.
VIRTIO_BLK_F_IDENTIFY etc? This way maybe I can use preprocessor
magic
to reduce duplication ...
It would even be great to only have a single point to add a feature
bit to.
So instead of
#define VIRTIO_BLK_F_IDENTIFY 1
You'd do
VIRTIO_FEATURE(BLK, IDENTIFY, 1)
which would add the feature including a lower case representation of
the same to an array you could use to evaluate features from.
By having magic like this we'd guarantee that new features will also
get exposed.
Also, I'd like these things to be saved in bits and not add a ton
of fields in device. Ideas how to do this?
Why?
But if you really want to because you want to save ram, just use int x:
1 style representations.
If you want to automatically construct a bitmap to compare to out of
the cmdline given feature bits, use the array I described above to
generate the bitmap in the init function.
Alex
Re: [Qemu-devel] Re: qdev property bug?
Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: On 12/14/09 10:44, Michael S. Tsirkin wrote: No, it did not even start booting the kernel. Just gave me blank screen. [ testing ] Oh. That is something completely different. A bug in the rom loader. It fails to fit both e1000 (default nic) and virtio-net boot roms into the option rom area and bails out (before loading seabios). vl.c doesn't check the return value and happily continues (without bios). Which doesn't work out very well ... With two identical nics the (single) rom fits and qemu boots. Hmm. Of course vl.c must be fixed to check the return value. Yes. Not sure how to deal with the rom size issue. The gPXE roms look quit big compared to the older roms we had. Hmm, it's a regression then ... How does real hw handle this? I'm pretty sure most servers these days use more option rom space than this. They usually have some onboard raid bios, external storage, on-board nic, pci nic, ... So there must be some way to just have more option rom space. Implementing anything else would just be a waste of time. It'd break again when ppl do device assignment. Alex
Re: [Qemu-devel] approaches to 3D virtualisation
On Saturday 12 December 2009, Dave Airlie wrote: > So I've been musing on the addition of some sort of 3D passthrough for > qemu (as I'm sure have lots of ppl) IIUC a typical graphics system consists of several operations: 1) Allocate space for data objects[2] on server[1]. 2) Upload data from client to server 3) Issue data processing commands that manipulate (combine/modify) data objects. For example a 3D rasterizer takes an image and sets of coordinates, and writes pixels to an image. 4) display data object to user. 5) Read data back to client. In modern systems this should almost never happen. I'd expect this to be the same for both 2D and 3D subsystems. The only real wart is that some 2D systems do not provide sufficient offload, and some processing is still done by the guest CPU. This means (5) is common, and you're effectively limited to a local implementation. With remote rendering the main difference is that you have a relatively high latency connection between client and server. If you have more than a few round-trips per frame you probably aren't going to get acceptable performance. IIUC this is why remote X connections perform so poorly, the protocol is effectively synchronous so the client must wait for a response from the server before sending the next command. In practical terms this means that the state of the graphics pipeline should not be guest visible. Considering the above pipeline, the only place where guest state is visible is (5). I'd expect that this almost never happens in normal circumstances. The fact the SLI/Crossfire setups can operate in AFR mode supports this theory. One prerequisite for isolating the graphics pipeline is that commands may not fail. I guess this may require step (1) be a synchronous operation. However steps (2), (3) and (4) should be fire-and-forget operations. If step (2) is defined as completing any time between the issue of the upload and the actual use then this allows both local zero-copy and remote explicit- upload implementations. A protocol that meets these requirements should be largely transport agnostic. While a full paravirtual interface may be desirable to squeeze the last bits of performance out, it should be possible to get acceptable performance over e.g. TCP, in the same way that the main benefit of virtio block/net drivers is simplicity and consistency rather than actual performance[3]. My understanding is that Chromium effectively implements the system described above, and I guess the VirtualBox implementation is just a custom transport backend and some modesetting tweaks. I have no specific knowledge of the VMware implementation. Once you have remote rendering the next problem is hotplug. IMO transparently migrating state is not an realistic option. This effectively required mirroring all of the server data on the guest. For source data (i.e. textures) this is fairly trivial. However for intermediate images (think redirected rendering of a 3D application window in a composited environment) this is not feasible. You could try to record the commands used to generate all intermediate data, however this also becomes infeasible. Command sequences may be large, and the original source data may no longer be available. Instead I suggest adding some sort of "damage" notification whereby the server can inform the client that data objects have been lost. When hotplug (switching to a different terminal) occurs we immediately complete all pending commands and report that all objects have been lost. The guest should then re- upload and regenerate as necessary and proceed to render the next frame. I'd expect that clients already have logic to do this as part of the VRAM handling for local video cards. As long as the guest never tries to read back the image, a null implementation is also trivial. Obviously all this is predicated on having a virtual display driver in the guest. For simple framebuffer devices, and actual VGA hardware, our initial premise that GPU state is not guest visible fails. In practice this means that there's little scope for doing remote server size acceleration, and you're reduced to implementing everything in the client and trying to optimize (2). Paul [1] I'm using X client/server terminology. The client is the guest OS and the server is the user's terminal. [2] Data objects include textures/bitmaps, vertex buffers, fragment programs, and probably command buffers. [3] Obviously if you emulate lame hardware like ne2k or IDE then performance will suck. However emulation of a high-end NIC of SCSI HBA should get within spitting distance of virtio.
Re: [Qemu-devel] [PATCH] correcting ARM CPSR register bit position comment
> -uint32_t VF; /* V is the bit 31. All other bits are undefined */ > +uint32_t VF; /* V is the bit 28. */ No. The original comment is correct. Paul
Re: [Qemu-devel] Guest bridge setup variations
On Friday 11 December 2009, Anthony Liguori wrote: > Arnd Bergmann wrote: > >> 3) given an fd, treat a vhost-style interface > > > > This could mean two things, not sure which one you mean. Either the > > file descriptor could be the vhost file descriptor, or the socket or tap > > file > > descriptor from above, with qemu operating on the vhost interface itself. > > > > Either option has its advantages, but I guess we should only implement > > one of the two to keep it simple. > > > > I was thinking the socket/tap descriptor. ok. > > Right. I wonder if this helper should integrate with netcf as an > > abstraction, > > or if we should rather do something generic. It may also be a good idea > > to let the helper decide which of the three options you listed to use > > and pass that back to qemu unless the user overrides it. The decision > > probably needs to be host specific, depending e.g. on the availability > > and version of tools (brctl, iproute, maybe tunctl, ...), the respective > > kernel modules (vhost, macvlan, bridge, tun, ...) and policy (VEPA, vlan, > > ebtables). Ideally the approach should be generic enough to work on > > other platforms (BSD, Solaris, Windows, ...). > > For helpers, I think I'd like to stick with what we currently support, > and then allow for a robust way for there to be independent projects > that implement their own helpers. For instance, I would love it if > netcf had a qemu network "plugin" helper. Moving netcf specific qemu helpers into the netcf project sounds good. I'm not sure what you mean with 'stick to what we currently support', do you mean with helpers that ship with qemu itself? That sounds reasonable, though I'd obviously like to make sure they also work with macvtap, which is currently not supported unless you pass an open file descriptor into -net tap,fd. > There's just too much in the networking space all wrapped up in layers > of policy decisions. I think it's time to move it out of qemu. Yes. Arnd
Re: [Qemu-devel] [PATCH] correcting ARM CPSR register bit position comment
On Mon, Dec 14, 2009 at 1:07 PM, Paul Brook wrote: >> - uint32_t VF; /* V is the bit 31. All other bits are undefined */ >> + uint32_t VF; /* V is the bit 28. */ > > No. The original comment is correct. And so that the answer is at least a bit useful: these fields are not directly mapped to CPSR; they are the results of computations and can then be translated to bits in CPSR (look for cpsr_read/cpsr_write in helper.c). If you want to see how VF is set, look for add_cc in op_helper.c. This is done this way to speed up simulation. HTH, Laurent
[Qemu-devel] [PATCH 03/11] pci: s/PCI_SUBVENDOR_ID/PCI_SUBSYSTEM_VENDOR_ID/g
To match Linux PCI register definition,
rename PCI_SUBVENDOR_ID to PCI_SUBSYSTEM_VENDOR_ID.
Signed-off-by: Isaku Yamahata
---
hw/pci.c |2 +-
hw/pci.h |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 344d72b..fb03ee2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -411,7 +411,7 @@ static int pci_set_default_subsystem_id(PCIDevice *pci_dev)
{
uint16_t *id;
-id = (void*)(&pci_dev->config[PCI_SUBVENDOR_ID]);
+id = (void*)(&pci_dev->config[PCI_SUBSYSTEM_VENDOR_ID]);
id[0] = cpu_to_le16(pci_default_sub_vendor_id);
id[1] = cpu_to_le16(pci_default_sub_device_id);
return 0;
diff --git a/hw/pci.h b/hw/pci.h
index d279e3f..0309674 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -155,7 +155,7 @@ typedef struct PCIIORegion {
#define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
#define PCI_REVISION0x08/* obsolete, use PCI_REVISION_ID */
-#define PCI_SUBVENDOR_ID0x2c/* obsolete, use
PCI_SUBSYSTEM_VENDOR_ID */
+#define PCI_SUBSYSTEM_VENDOR_ID 0x2c
#define PCI_SUBDEVICE_ID0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
/* Bits in the PCI Status Register (PCI 2.3 spec) */
--
1.6.5.4
[Qemu-devel] [PATCH 02/11] pci: clean up pci_bar_address()
make pci_bar_address independent of PCI_BAR_UNMAPPED value.
PCI_BAR_UNMAPPED could be arbitrary value which doesn't match
possible pci bar. So == PCI_BAR_UNMAPPED check is not good.
This patch cleans it up.
Signed-off-by: Isaku Yamahata
---
hw/pci.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index aed3a24..344d72b 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -857,8 +857,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
/* XXX: as we cannot support really dynamic
mappings, we handle specific values as invalid
mappings. */
-if (last_addr <= new_addr || new_addr == 0 ||
-last_addr == PCI_BAR_UNMAPPED) {
+if (last_addr <= new_addr || new_addr == 0 || last_addr == ~(pcibus_t)0) {
return PCI_BAR_UNMAPPED;
}
--
1.6.5.4
[Qemu-devel] [PATCH 00/11] various pci clean ups.
This patch series is for various somewhat atrandom clean up.
Michael, this patch series possibly conflicts with your cleanups.
Which patch should I rebase to?
Isaku Yamahata (11):
pci: remove PCIBus::config_reg.
pci: clean up pci_bar_address()
pci: s/PCI_SUBVENDOR_ID/PCI_SUBSYSTEM_VENDOR_ID/g
pci: remove PCI_REVISION and PCI_SUBDEVICE_ID.
pci: import Linux pci_regs.h
pci: use pci_regs.h
gt64xxx: remove gt64120_{read, write}_config().
acpi: use range helper function.
piix_pci: define symbolic value for PAM0, PAM6 and SMRAM.
piix_pci: use range helper function
msix: use range helper function.
hw/acpi.c |2 +-
hw/gt64xxx.c | 13 +-
hw/msix.c |2 +-
hw/pci.c |6 +-
hw/pci.h | 74 +--
hw/pci_regs.h | 665 +
hw/piix_pci.c | 15 +-
7 files changed, 684 insertions(+), 93 deletions(-)
create mode 100644 hw/pci_regs.h
[Qemu-devel] [PATCH 08/11] acpi: use range helper function.
use range helper function in pm_write_config().
Signed-off-by: Isaku Yamahata
---
hw/acpi.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index 9a69e7d..ad72297 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -437,7 +437,7 @@ static void pm_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
pci_default_write_config(d, address, val, len);
-if (address == 0x80)
+if (range_covers_byte(address, len, 0x80))
pm_io_space_update((PIIX4PMState *)d);
}
--
1.6.5.4
[Qemu-devel] [PATCH 06/11] pci: use pci_regs.h
include pci_regs.h and remove duplicated defines.
Signed-off-by: Isaku Yamahata
---
hw/pci.h | 72 ++---
1 files changed, 3 insertions(+), 69 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 91f3809..b5e7abb 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -94,76 +94,10 @@ typedef struct PCIIORegion {
#define PCI_ROM_SLOT 6
#define PCI_NUM_REGIONS 7
-/* Declarations from linux/pci_regs.h */
-#define PCI_VENDOR_ID 0x00/* 16 bits */
-#define PCI_DEVICE_ID 0x02/* 16 bits */
-#define PCI_COMMAND0x04/* 16 bits */
-#define PCI_COMMAND_IO0x1 /* Enable response in I/O space
*/
-#define PCI_COMMAND_MEMORY0x2 /* Enable response in Memory space */
-#define PCI_COMMAND_MASTER0x4 /* Enable bus master */
-#define PCI_COMMAND_INTX_DISABLE 0x400 /* INTx Emulation Disable */
-#define PCI_STATUS 0x06/* 16 bits */
-#define PCI_STATUS_INTERRUPT 0x08
-#define PCI_REVISION_ID 0x08/* 8 bits */
-#define PCI_CLASS_PROG 0x09/* Reg. Level Programming Interface */
-#define PCI_CLASS_DEVICE0x0a/* Device class */
-#define PCI_CACHE_LINE_SIZE0x0c/* 8 bits */
-#define PCI_LATENCY_TIMER 0x0d/* 8 bits */
-#define PCI_HEADER_TYPE 0x0e/* 8 bits */
-#define PCI_HEADER_TYPE_NORMAL0
-#define PCI_HEADER_TYPE_BRIDGE1
-#define PCI_HEADER_TYPE_CARDBUS 2
+#include "pci_regs.h"
+
+/* PCI HEADER_TYPE */
#define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80
-#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */
-#define PCI_BASE_ADDRESS_SPACE_IO 0x01
-#define PCI_BASE_ADDRESS_SPACE_MEMORY 0x00
-#define PCI_BASE_ADDRESS_MEM_TYPE_64 0x04/* 64 bit address */
-#define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08/* prefetchable? */
-#define PCI_PRIMARY_BUS0x18/* Primary bus number */
-#define PCI_SECONDARY_BUS 0x19/* Secondary bus number */
-#define PCI_SUBORDINATE_BUS0x1a/* Highest bus number behind the bridge
*/
-#define PCI_IO_BASE 0x1c/* I/O range behind the bridge */
-#define PCI_IO_LIMIT0x1d
-#define PCI_IO_RANGE_TYPE_32 0x01
-#define PCI_IO_RANGE_MASK (~0x0fUL)
-#define PCI_SEC_STATUS 0x1e/* Secondary status register, only bit
14 used */
-#define PCI_MEMORY_BASE 0x20/* Memory range behind */
-#define PCI_MEMORY_LIMIT0x22
-#define PCI_MEMORY_RANGE_MASK (~0x0fUL)
-#define PCI_PREF_MEMORY_BASE0x24/* Prefetchable memory range behind */
-#define PCI_PREF_MEMORY_LIMIT 0x26
-#define PCI_PREF_RANGE_MASK(~0x0fUL)
-#define PCI_PREF_RANGE_TYPE_64 0x01
-#define PCI_PREF_BASE_UPPER32 0x28/* Upper half of prefetchable memory
range */
-#define PCI_PREF_LIMIT_UPPER32 0x2c
-#define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */
-#define PCI_SUBSYSTEM_ID0x2e/* 16 bits */
-#define PCI_ROM_ADDRESS0x30/* Bits 31..11 are address,
10..1 reserved */
-#define PCI_ROM_ADDRESS_ENABLE0x01
-#define PCI_IO_BASE_UPPER16 0x30/* Upper half of I/O addresses */
-#define PCI_IO_LIMIT_UPPER160x32
-#define PCI_CAPABILITY_LIST0x34/* Offset of first capability list
entry */
-#define PCI_ROM_ADDRESS1 0x38/* Same as PCI_ROM_ADDRESS, but for
htype 1 */
-#define PCI_INTERRUPT_LINE 0x3c/* 8 bits */
-#define PCI_INTERRUPT_PIN 0x3d/* 8 bits */
-#define PCI_MIN_GNT0x3e/* 8 bits */
-#define PCI_BRIDGE_CONTROL 0x3e
-#define PCI_MAX_LAT0x3f/* 8 bits */
-
-/* Capability lists */
-#define PCI_CAP_LIST_ID0 /* Capability ID */
-#define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
-
-#define PCI_SUBSYSTEM_VENDOR_ID 0x2c
-
-/* Bits in the PCI Status Register (PCI 2.3 spec) */
-#define PCI_STATUS_RESERVED1 0x007
-#define PCI_STATUS_INT_STATUS 0x008
-#define PCI_STATUS_CAP_LIST0x010
-#define PCI_STATUS_66MHZ 0x020
-#define PCI_STATUS_RESERVED2 0x040
-#define PCI_STATUS_FAST_BACK 0x080
-#define PCI_STATUS_DEVSEL 0x600
#define PCI_STATUS_RESERVED_MASK_LO (PCI_STATUS_RESERVED1 | \
PCI_STATUS_INT_STATUS | PCI_STATUS_CAPABILITIES | \
--
1.6.5.4
[Qemu-devel] [PATCH 04/11] pci: remove PCI_REVISION and PCI_SUBDEVICE_ID.
There is no user and they're obsolete. So remove them.
Signed-off-by: Isaku Yamahata
---
hw/pci.h |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 0309674..91f3809 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -154,9 +154,7 @@ typedef struct PCIIORegion {
#define PCI_CAP_LIST_ID0 /* Capability ID */
#define PCI_CAP_LIST_NEXT 1 /* Next capability in the list */
-#define PCI_REVISION0x08/* obsolete, use PCI_REVISION_ID */
#define PCI_SUBSYSTEM_VENDOR_ID 0x2c
-#define PCI_SUBDEVICE_ID0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
/* Bits in the PCI Status Register (PCI 2.3 spec) */
#define PCI_STATUS_RESERVED1 0x007
--
1.6.5.4
[Qemu-devel] [PATCH 09/11] piix_pci: define symbolic value for PAM0, PAM6 and SMRAM.
Define symbolic value in i440fx configuration space
for 0x59, 0x5f and 0x7f and use them.
Signed-off-by: Isaku Yamahata
---
hw/piix_pci.c | 13 +
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 1b67475..7bbaf50 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -44,6 +44,10 @@ struct PCII440FXState {
PIIX3State *piix3;
};
+#define I440FX_PAM0 0x59
+#define I440FX_PAM6 0x5f
+#define I440FX_SMRAM0x72
+
static void piix3_set_irq(void *opaque, int irq_num, int level);
/* return the global irq number corresponding to a given device irq
@@ -88,12 +92,12 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
int i, r;
uint32_t smram, addr;
-update_pam(d, 0xf, 0x10, (d->dev.config[0x59] >> 4) & 3);
+update_pam(d, 0xf, 0x10, (d->dev.config[I440FX_PAM0] >> 4) & 3);
for(i = 0; i < 12; i++) {
r = (d->dev.config[(i >> 1) + 0x5a] >> ((i & 1) * 4)) & 3;
update_pam(d, 0xc + 0x4000 * i, 0xc + 0x4000 * (i + 1), r);
}
-smram = d->dev.config[0x72];
+smram = d->dev.config[I440FX_SMRAM];
if ((d->smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
cpu_register_physical_memory(0xa, 0x2, 0xa);
} else {
@@ -132,7 +136,8 @@ static void i440fx_write_config(PCIDevice *dev,
/* XXX: implement SMRAM.D_LOCK */
pci_default_write_config(dev, address, val, len);
-if ((address >= 0x59 && address <= 0x5f) || address == 0x72)
+if ((address >= I440FX_PAM0 && address <= I440FX_PAM6) ||
+address == I440FX_SMRAM)
i440fx_update_memory_mappings(d);
}
@@ -196,7 +201,7 @@ static int i440fx_initfn(PCIDevice *dev)
pci_config_set_class(d->dev.config, PCI_CLASS_BRIDGE_HOST);
d->dev.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
-d->dev.config[0x72] = 0x02; /* SMRAM */
+d->dev.config[I440FX_SMRAM] = 0x02; /* SMRAM */
return 0;
}
--
1.6.5.4
[Qemu-devel] [PATCH 01/11] pci: remove PCIBus::config_reg.
PCIBus::config_reg isn't used anymore, so remove it.
Signed-off-by: Isaku Yamahata
---
hw/pci.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 404eead..aed3a24 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@ struct PCIBus {
pci_set_irq_fn set_irq;
pci_map_irq_fn map_irq;
pci_hotplug_fn hotplug;
-uint32_t config_reg; /* XXX: suppress */
void *irq_opaque;
PCIDevice *devices[256];
PCIDevice *parent_dev;
--
1.6.5.4
[Qemu-devel] [PATCH 10/11] piix_pci: use range helper function
use range helper function in i440fx_write_config().
Signed-off-by: Isaku Yamahata
---
hw/piix_pci.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7bbaf50..304f84a 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -136,9 +136,11 @@ static void i440fx_write_config(PCIDevice *dev,
/* XXX: implement SMRAM.D_LOCK */
pci_default_write_config(dev, address, val, len);
-if ((address >= I440FX_PAM0 && address <= I440FX_PAM6) ||
-address == I440FX_SMRAM)
+if (ranges_overlap(address, len,
+ I440FX_PAM0, I440FX_PAM6 - I440FX_PAM0 + 1) ||
+range_covers_byte(address, len, I440FX_SMRAM)) {
i440fx_update_memory_mappings(d);
+}
}
static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
--
1.6.5.4
[Qemu-devel] [PATCH 05/11] pci: import Linux pci_regs.h
Import Linux pci_regs.h. Later PCI register definitions in pci.h will be eliminated. Signed-off-by: Isaku Yamahata --- hw/pci_regs.h | 665 + 1 files changed, 665 insertions(+), 0 deletions(-) create mode 100644 hw/pci_regs.h diff --git a/hw/pci_regs.h b/hw/pci_regs.h new file mode 100644 index 000..dd0bed4 --- /dev/null +++ b/hw/pci_regs.h @@ -0,0 +1,665 @@ +/* + * pci_regs.h + * + * PCI standard defines + * Copyright 1994, Drew Eckhardt + * Copyright 1997--1999 Martin Mares + * + * For more information, please consult the following manuals (look at + * http://www.pcisig.com/ for how to get them): + * + * PCI BIOS Specification + * PCI Local Bus Specification + * PCI to PCI Bridge Specification + * PCI System Design Guide + * + * For hypertransport information, please consult the following manuals + * from http://www.hypertransport.org + * + * The Hypertransport I/O Link Specification + */ + +#ifndef LINUX_PCI_REGS_H +#define LINUX_PCI_REGS_H + +/* + * Under PCI, each device has 256 bytes of configuration address space, + * of which the first 64 bytes are standardized as follows: + */ +#define PCI_VENDOR_ID 0x00/* 16 bits */ +#define PCI_DEVICE_ID 0x02/* 16 bits */ +#define PCI_COMMAND0x04/* 16 bits */ +#define PCI_COMMAND_IO0x1 /* Enable response in I/O space */ +#define PCI_COMMAND_MEMORY0x2 /* Enable response in Memory space */ +#define PCI_COMMAND_MASTER0x4 /* Enable bus mastering */ +#define PCI_COMMAND_SPECIAL 0x8 /* Enable response to special cycles */ +#define PCI_COMMAND_INVALIDATE0x10/* Use memory write and invalidate */ +#define PCI_COMMAND_VGA_PALETTE 0x20 /* Enable palette snooping */ +#define PCI_COMMAND_PARITY0x40/* Enable parity checking */ +#define PCI_COMMAND_WAIT 0x80/* Enable address/data stepping */ +#define PCI_COMMAND_SERR 0x100 /* Enable SERR */ +#define PCI_COMMAND_FAST_BACK 0x200 /* Enable back-to-back writes */ +#define PCI_COMMAND_INTX_DISABLE 0x400 /* INTx Emulation Disable */ + +#define PCI_STATUS 0x06/* 16 bits */ +#define PCI_STATUS_INTERRUPT 0x08/* Interrupt status */ +#define PCI_STATUS_CAP_LIST 0x10/* Support Capability List */ +#define PCI_STATUS_66MHZ 0x20/* Support 66 Mhz PCI 2.1 bus */ +#define PCI_STATUS_UDF0x40/* Support User Definable Features [obsolete] */ +#define PCI_STATUS_FAST_BACK 0x80/* Accept fast-back to back */ +#define PCI_STATUS_PARITY 0x100 /* Detected parity error */ +#define PCI_STATUS_DEVSEL_MASK0x600 /* DEVSEL timing */ +#define PCI_STATUS_DEVSEL_FAST0x000 +#define PCI_STATUS_DEVSEL_MEDIUM 0x200 +#define PCI_STATUS_DEVSEL_SLOW0x400 +#define PCI_STATUS_SIG_TARGET_ABORT 0x800 /* Set on target abort */ +#define PCI_STATUS_REC_TARGET_ABORT 0x1000 /* Master ack of " */ +#define PCI_STATUS_REC_MASTER_ABORT 0x2000 /* Set on master abort */ +#define PCI_STATUS_SIG_SYSTEM_ERROR 0x4000 /* Set when we drive SERR */ +#define PCI_STATUS_DETECTED_PARITY0x8000 /* Set on parity error */ + +#define PCI_CLASS_REVISION 0x08/* High 24 bits are class, low 8 revision */ +#define PCI_REVISION_ID0x08/* Revision ID */ +#define PCI_CLASS_PROG 0x09/* Reg. Level Programming Interface */ +#define PCI_CLASS_DEVICE 0x0a/* Device class */ + +#define PCI_CACHE_LINE_SIZE0x0c/* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d/* 8 bits */ +#define PCI_HEADER_TYPE0x0e/* 8 bits */ +#define PCI_HEADER_TYPE_NORMAL0 +#define PCI_HEADER_TYPE_BRIDGE1 +#define PCI_HEADER_TYPE_CARDBUS 2 + +#define PCI_BIST 0x0f/* 8 bits */ +#define PCI_BIST_CODE_MASK0x0f/* Return result */ +#define PCI_BIST_START0x40/* 1 to start BIST, 2 secs or less */ +#define PCI_BIST_CAPABLE 0x80/* 1 if BIST capable */ + +/* + * Base addresses specify locations in memory or I/O space. + * Decoded size can be determined by writing a value of + * 0x to the register, and reading it back. Only + * 1 bits are decoded. + */ +#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */ +#define PCI_BASE_ADDRESS_1 0x14/* 32 bits [htype 0,1 only] */ +#define PCI_BASE_ADDRESS_2 0x18/* 32 bits [htype 0 only] */ +#define PCI_BASE_ADDRESS_3 0x1c/* 32 bits */ +#define PCI_BASE_ADDRESS_4 0x20/* 32 bits */ +#define PCI_BASE_ADDRESS_5 0x24/* 32 bits */ +#define PCI_BASE_ADDRESS_SPACE0x01/* 0 = memory, 1 = I/O */ +#define PCI_BASE_ADDRESS_SPACE_IO 0x01 +#define PCI_BASE_ADDRESS_SPACE_MEMORY 0x00 +#define PCI_BASE_ADDRESS_MEM_TYPE_MASK0x06 +#define PCI_BASE_ADDRESS_MEM_TYPE_32 0x00/* 32 bit
[Qemu-devel] [PATCH 11/11] msix: use range helper function.
use range helper function in msix_write_config().
Signed-off-by: Isaku Yamahata
---
hw/msix.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/msix.c b/hw/msix.c
index 0baedef..2ca0900 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -175,7 +175,7 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET;
int vector;
-if (addr + len <= enable_pos || addr > enable_pos) {
+if (!range_covers_byte(addr, len, enable_pos)) {
return;
}
--
1.6.5.4
[Qemu-devel] [PATCH 07/11] gt64xxx: remove gt64120_{read, write}_config().
They call only pci_default_{read, write}_config().
So they aren't necessary.
Signed-off-by: Isaku Yamahata
---
hw/gt64xxx.c | 13 +
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index fb7f5bd..c8034e2 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1082,17 +1082,6 @@ static void gt64120_reset(void *opaque)
gt64120_pci_mapping(s);
}
-static uint32_t gt64120_read_config(PCIDevice *d, uint32_t address, int len)
-{
-return pci_default_read_config(d, address, len);
-}
-
-static void gt64120_write_config(PCIDevice *d, uint32_t address, uint32_t val,
- int len)
-{
-pci_default_write_config(d, address, val, len);
-}
-
static void gt64120_save(QEMUFile* f, void *opaque)
{
PCIDevice *d = opaque;
@@ -1125,7 +1114,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
pic, 144, 4);
s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
-0, gt64120_read_config, gt64120_write_config);
+0, NULL, NULL);
/* FIXME: Malta specific hw assumptions ahead */
--
1.6.5.4
[Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On Mon, Dec 14, 2009 at 12:37:29PM +0100, Gerd Hoffmann wrote:
> On 12/14/09 12:10, Michael S. Tsirkin wrote:
>> On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
>> for block:
>> if (strcmp(s->serial_str, "0"))
>> features |= 1<< VIRTIO_BLK_F_IDENTIFY;
>>
>> if (bdrv_is_read_only(s->bs))
>> features |= 1<< VIRTIO_BLK_F_RO;
>
> Sure you want these be configurable?
>
>> Also, I'd like these things to be saved in bits and not add a ton
>> of fields in device. Ideas how to do this?
>
> I guess you only want disable features?
>
> You could have a bitmap property then, which accepts names for the bits.
> It would need a table like this ...
>
>char *bitnames[] = {
> [ VIRTIO_BLK_F_IDENTIFY ] = "blk-identify",
> [ VIRTIO_BLK_F_RO [ = "blk-ro",
> [ ... ]
>};
>
> Then the property parser would accepts strings such as 'bit1|bit2' and
> you can have
>
> -device 'virtio-blk-pci,disable=blk-identify|ring-indirect'
>
> The driver will just do 'vdev->host_features &= ~disable'.
>
> cheers,
> Gerd
Excellent.
--
MST
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > > Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > >> On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: >>> On 12/14/09 10:44, Michael S. Tsirkin wrote: No, it did not even start booting the kernel. Just gave me blank screen. >>> >>> [ testing ] >>> >>> Oh. That is something completely different. A bug in the rom >>> loader. >>> It fails to fit both e1000 (default nic) and virtio-net boot roms >>> into >>> the option rom area and bails out (before loading seabios). vl.c >>> doesn't check the return value and happily continues (without bios). >>> Which doesn't work out very well ... >>> >>> With two identical nics the (single) rom fits and qemu boots. >>> >>> Hmm. Of course vl.c must be fixed to check the return value. >> >> Yes. >> >>> Not sure how to deal with the rom size issue. The gPXE roms look >>> quit >>> big compared to the older roms we had. >> >> Hmm, it's a regression then ... > > How does real hw handle this? I'm pretty sure most servers these days > use more option rom space than this. They usually have some onboard raid > bios, external storage, on-board nic, pci nic, ... Real hardware might do several things I know about - option rom is typically small. - option rom is not loaded always (BIOS option), or not for all cards. There are might be other tricks. > So there must be some way to just have more option rom space. What do you mean? > Implementing anything else would just be a waste of time. It'd break > again when ppl do device assignment. > > Alex We need some solution for 0.12 though IMO. This does not need to address device assignment, but it must be simple. -- MST
[Qemu-devel] Re: [PATCH 11/11] msix: use range helper function.
On Mon, Dec 14, 2009 at 09:48:26PM +0900, Isaku Yamahata wrote:
> use range helper function in msix_write_config().
>
> Signed-off-by: Isaku Yamahata
Acked-by: Michael S. Tsirkin
> ---
> hw/msix.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/msix.c b/hw/msix.c
> index 0baedef..2ca0900 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -175,7 +175,7 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
> unsigned enable_pos = dev->msix_cap + MSIX_CONTROL_OFFSET;
> int vector;
>
> -if (addr + len <= enable_pos || addr > enable_pos) {
> +if (!range_covers_byte(addr, len, enable_pos)) {
> return;
> }
>
> --
> 1.6.5.4
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
Gerd Hoffmann writes:
> On 12/14/09 12:10, Michael S. Tsirkin wrote:
>> On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
>> for block:
>> if (strcmp(s->serial_str, "0"))
>> features |= 1<< VIRTIO_BLK_F_IDENTIFY;
>>
>> if (bdrv_is_read_only(s->bs))
>> features |= 1<< VIRTIO_BLK_F_RO;
>
> Sure you want these be configurable?
>
>> Also, I'd like these things to be saved in bits and not add a ton
>> of fields in device. Ideas how to do this?
>
> I guess you only want disable features?
>
> You could have a bitmap property then, which accepts names for the
> bits. It would need a table like this ...
>
>char *bitnames[] = {
> [ VIRTIO_BLK_F_IDENTIFY ] = "blk-identify",
> [ VIRTIO_BLK_F_RO [ = "blk-ro",
> [ ... ]
>};
>
> Then the property parser would accepts strings such as 'bit1|bit2' and
> you can have
>
> -device 'virtio-blk-pci,disable=blk-identify|ring-indirect'
>
> The driver will just do 'vdev->host_features &= ~disable'.
Use of '|' in option argument syntax is user-hostile, because it
requires quoting in the shell. What about '+'?
[Qemu-devel] [PATCH] A different way to ask for readonly drive
Hi,
After feedback from Red Hat guys, I've decided to slightly modify the approach
to drive's readonly.
The new approach also addresses the silent fall-back to open the drives' file
as read-only when read-write fails
(permission denied) that causes unexpected behavior.
Instead of the 'readonly' boolean flag, another flag introduced (a
replacement), 'read_write' with three values [on|off|try]:
read_write=on : open with read and write permission, no fall-back to read-only
read_write=off: open with read-only permission
read_write=try: open with read and write permission and if fails, fall-back to
read-only (the default if nothing specified)
Suggestions for better naming for flag/values welcomed.
I've tried to explicitly pass the required flags for the bdrv_open function in
callers, but probably missed some.
Naphtali
Signed-off-by: Naphtali Sprei
---
block.c | 29 +
block.h |7 +--
hw/xen_disk.c |3 ++-
monitor.c |2 +-
qemu-config.c |4 ++--
qemu-img.c| 14 --
qemu-nbd.c|2 +-
vl.c | 42 +-
8 files changed, 69 insertions(+), 34 deletions(-)
diff --git a/block.c b/block.c
index 3f3496e..75788ca 100644
--- a/block.c
+++ b/block.c
@@ -356,7 +356,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags)
int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv)
{
-int ret, open_flags, try_rw;
+int ret, open_flags;
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
@@ -378,7 +378,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename,
int flags,
/* if there is a backing file, use it */
bs1 = bdrv_new("");
-ret = bdrv_open2(bs1, filename, 0, drv);
+ret = bdrv_open2(bs1, filename, BDRV_O_RDONLY, drv);
if (ret < 0) {
bdrv_delete(bs1);
return ret;
@@ -445,19 +445,22 @@ int bdrv_open2(BlockDriverState *bs, const char
*filename, int flags,
bs->enable_write_cache = 1;
/* Note: for compatibility, we open disk image files as RDWR, and
- RDONLY as fallback */
-try_rw = !bs->read_only || bs->is_temporary;
-if (!(flags & BDRV_O_FILE))
-open_flags = (try_rw ? BDRV_O_RDWR : 0) |
-(flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
-else
+ RDONLY as fallback (if flag BDRV_O_RO_FBCK is on) */
+bs->read_only = BDRV_FLAGS_IS_RO(flags);
+if (!(flags & BDRV_O_FILE)) {
+open_flags = (flags & (BDRV_O_ACCESS |
BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
+if (bs->is_temporary) { /* snapshot */
+open_flags |= BDRV_O_RDWR;
+}
+} else
open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv))
ret = -ENOTSUP;
else
ret = drv->bdrv_open(bs, filename, open_flags);
-if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
-ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
+if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE) &&
+(flags & BDRV_O_RO_FBCK)) {
+ret = drv->bdrv_open(bs, filename, (open_flags & ~BDRV_O_RDWR) |
BDRV_O_RDONLY);
bs->read_only = 1;
}
if (ret < 0) {
@@ -481,12 +484,14 @@ int bdrv_open2(BlockDriverState *bs, const char
*filename, int flags,
/* if there is a backing file, use it */
BlockDriver *back_drv = NULL;
bs->backing_hd = bdrv_new("");
-/* pass on read_only property to the backing_hd */
-bs->backing_hd->read_only = bs->read_only;
path_combine(backing_filename, sizeof(backing_filename),
filename, bs->backing_file);
if (bs->backing_format[0] != '\0')
back_drv = bdrv_find_format(bs->backing_format);
+/* pass on ro flags to the backing_hd */
+bs->backing_hd->read_only = BDRV_FLAGS_IS_RO(flags);
+open_flags &= ~BDRV_O_ACCESS;
+open_flags |= (flags & BDRV_O_ACCESS);
ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
back_drv);
if (ret < 0) {
diff --git a/block.h b/block.h
index fa51ddf..b32c6a4 100644
--- a/block.h
+++ b/block.h
@@ -28,8 +28,9 @@ typedef struct QEMUSnapshotInfo {
} QEMUSnapshotInfo;
#define BDRV_O_RDONLY 0x
-#define BDRV_O_RDWR0x0002
-#define BDRV_O_ACCESS 0x0003
+#define BDRV_O_RDWR0x0001
+#define BDRV_O_ACCESS 0x0001
+#define BDRV_O_RO_FBCK 0x0002
#define BDRV_O_CREAT 0x0004 /* create an empty file */
#define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes
in a snapshot */
#define BDRV_O_FILE0x0010 /* open as a raw file (do not try to
@@ -41,6 +42,8 @@ typedef struct QEMUSnapshotInfo {
#define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of
Re: [Qemu-devel] Re: qdev property bug?
Michael S. Tsirkin wrote: > On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > >> Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : >> >> >>> On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: >>> On 12/14/09 10:44, Michael S. Tsirkin wrote: > No, it did not even start booting the kernel. Just gave me blank > screen. > [ testing ] Oh. That is something completely different. A bug in the rom loader. It fails to fit both e1000 (default nic) and virtio-net boot roms into the option rom area and bails out (before loading seabios). vl.c doesn't check the return value and happily continues (without bios). Which doesn't work out very well ... With two identical nics the (single) rom fits and qemu boots. Hmm. Of course vl.c must be fixed to check the return value. >>> Yes. >>> >>> Not sure how to deal with the rom size issue. The gPXE roms look quit big compared to the older roms we had. >>> Hmm, it's a regression then ... >>> >> How does real hw handle this? I'm pretty sure most servers these days >> use more option rom space than this. They usually have some onboard raid >> bios, external storage, on-board nic, pci nic, ... >> > > Real hardware might do several things I know about > - option rom is typically small. > - option rom is not loaded always (BIOS option), or not for all cards. > There are might be other tricks. > There are probably other tricks. I was booting up a machine that had like 5 options roms going through their initialization that all weren't exactly small. >> So there must be some way to just have more option rom space. >> > > What do you mean? > Well, what's keeping us from having 5 MB of option roms? >> Implementing anything else would just be a waste of time. It'd break >> again when ppl do device assignment. >> >> Alex >> > > We need some solution for 0.12 though IMO. > This does not need to address device assignment, > but it must be simple. > Agreed. If there is a solution that gives us the chance to support an arbitrary number of option roms that wouldn't take forever to implement, I'd rather take that one though. Alex
[Qemu-devel] Re: [PATCH 02/11] pci: clean up pci_bar_address()
On Mon, Dec 14, 2009 at 09:48:17PM +0900, Isaku Yamahata wrote:
> make pci_bar_address independent of PCI_BAR_UNMAPPED value.
> PCI_BAR_UNMAPPED could be arbitrary value which doesn't match
> possible pci bar. So == PCI_BAR_UNMAPPED check is not good.
> This patch cleans it up.
>
> Signed-off-by: Isaku Yamahata
I would rather we removed this hack altogether.
I guess proper fix requires Avi's patch to do BAR mapping
from pci.c, and I think we should do just this.
There are no special values in bar
assignment, and guests are free to perform sizing any way they want.
Especially for 64 bit BARs, sizing for high bits is
done separately from low bits.
Also - I think this in fact changes logic.
On 64 bit qemu, we used to compare to 0x
and now we compare to 0x.
> ---
> hw/pci.c |3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index aed3a24..344d72b 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -857,8 +857,7 @@ static pcibus_t pci_bar_address(PCIDevice *d,
> /* XXX: as we cannot support really dynamic
> mappings, we handle specific values as invalid
> mappings. */
> -if (last_addr <= new_addr || new_addr == 0 ||
> -last_addr == PCI_BAR_UNMAPPED) {
> +if (last_addr <= new_addr || new_addr == 0 || last_addr == ~(pcibus_t)0)
> {
> return PCI_BAR_UNMAPPED;
> }
>
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 03/11] pci: s/PCI_SUBVENDOR_ID/PCI_SUBSYSTEM_VENDOR_ID/g
On Mon, Dec 14, 2009 at 09:48:18PM +0900, Isaku Yamahata wrote:
> To match Linux PCI register definition,
> rename PCI_SUBVENDOR_ID to PCI_SUBSYSTEM_VENDOR_ID.
>
> Signed-off-by: Isaku Yamahata
Acked-by: Michael S. Tsirkin
> ---
> hw/pci.c |2 +-
> hw/pci.h |2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 344d72b..fb03ee2 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -411,7 +411,7 @@ static int pci_set_default_subsystem_id(PCIDevice
> *pci_dev)
> {
> uint16_t *id;
>
> -id = (void*)(&pci_dev->config[PCI_SUBVENDOR_ID]);
> +id = (void*)(&pci_dev->config[PCI_SUBSYSTEM_VENDOR_ID]);
> id[0] = cpu_to_le16(pci_default_sub_vendor_id);
> id[1] = cpu_to_le16(pci_default_sub_device_id);
> return 0;
> diff --git a/hw/pci.h b/hw/pci.h
> index d279e3f..0309674 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -155,7 +155,7 @@ typedef struct PCIIORegion {
> #define PCI_CAP_LIST_NEXT1 /* Next capability in the list */
>
> #define PCI_REVISION0x08/* obsolete, use PCI_REVISION_ID */
> -#define PCI_SUBVENDOR_ID0x2c/* obsolete, use
> PCI_SUBSYSTEM_VENDOR_ID */
> +#define PCI_SUBSYSTEM_VENDOR_ID 0x2c
> #define PCI_SUBDEVICE_ID0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
>
> /* Bits in the PCI Status Register (PCI 2.3 spec) */
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 06/11] pci: use pci_regs.h
On Mon, Dec 14, 2009 at 09:48:21PM +0900, Isaku Yamahata wrote:
> include pci_regs.h and remove duplicated defines.
>
> Signed-off-by: Isaku Yamahata
Good stuff.
Acked-by: Michael S. Tsirkin
> ---
> hw/pci.h | 72 ++---
> 1 files changed, 3 insertions(+), 69 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 91f3809..b5e7abb 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -94,76 +94,10 @@ typedef struct PCIIORegion {
> #define PCI_ROM_SLOT 6
> #define PCI_NUM_REGIONS 7
>
> -/* Declarations from linux/pci_regs.h */
> -#define PCI_VENDOR_ID0x00/* 16 bits */
> -#define PCI_DEVICE_ID0x02/* 16 bits */
> -#define PCI_COMMAND 0x04/* 16 bits */
> -#define PCI_COMMAND_IO 0x1 /* Enable response in I/O space
> */
> -#define PCI_COMMAND_MEMORY 0x2 /* Enable response in Memory space */
> -#define PCI_COMMAND_MASTER 0x4 /* Enable bus master */
> -#define PCI_COMMAND_INTX_DISABLE 0x400 /* INTx Emulation Disable */
> -#define PCI_STATUS 0x06/* 16 bits */
> -#define PCI_STATUS_INTERRUPT 0x08
> -#define PCI_REVISION_ID 0x08/* 8 bits */
> -#define PCI_CLASS_PROG 0x09/* Reg. Level Programming
> Interface */
> -#define PCI_CLASS_DEVICE0x0a/* Device class */
> -#define PCI_CACHE_LINE_SIZE 0x0c/* 8 bits */
> -#define PCI_LATENCY_TIMER0x0d/* 8 bits */
> -#define PCI_HEADER_TYPE 0x0e/* 8 bits */
> -#define PCI_HEADER_TYPE_NORMAL 0
> -#define PCI_HEADER_TYPE_BRIDGE 1
> -#define PCI_HEADER_TYPE_CARDBUS 2
> +#include "pci_regs.h"
> +
> +/* PCI HEADER_TYPE */
> #define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80
> -#define PCI_BASE_ADDRESS_0 0x10/* 32 bits */
> -#define PCI_BASE_ADDRESS_SPACE_IO 0x01
> -#define PCI_BASE_ADDRESS_SPACE_MEMORY 0x00
> -#define PCI_BASE_ADDRESS_MEM_TYPE_640x04/* 64 bit address */
> -#define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08/* prefetchable? */
> -#define PCI_PRIMARY_BUS 0x18/* Primary bus number */
> -#define PCI_SECONDARY_BUS0x19/* Secondary bus number */
> -#define PCI_SUBORDINATE_BUS 0x1a/* Highest bus number behind the bridge
> */
> -#define PCI_IO_BASE 0x1c/* I/O range behind the bridge */
> -#define PCI_IO_LIMIT0x1d
> -#define PCI_IO_RANGE_TYPE_320x01
> -#define PCI_IO_RANGE_MASK (~0x0fUL)
> -#define PCI_SEC_STATUS 0x1e/* Secondary status register,
> only bit 14 used */
> -#define PCI_MEMORY_BASE 0x20/* Memory range behind */
> -#define PCI_MEMORY_LIMIT0x22
> -#define PCI_MEMORY_RANGE_MASK (~0x0fUL)
> -#define PCI_PREF_MEMORY_BASE0x24/* Prefetchable memory range behind
> */
> -#define PCI_PREF_MEMORY_LIMIT 0x26
> -#define PCI_PREF_RANGE_MASK(~0x0fUL)
> -#define PCI_PREF_RANGE_TYPE_64 0x01
> -#define PCI_PREF_BASE_UPPER32 0x28/* Upper half of prefetchable memory
> range */
> -#define PCI_PREF_LIMIT_UPPER32 0x2c
> -#define PCI_SUBSYSTEM_VENDOR_ID 0x2c/* 16 bits */
> -#define PCI_SUBSYSTEM_ID0x2e/* 16 bits */
> -#define PCI_ROM_ADDRESS 0x30/* Bits 31..11 are address,
> 10..1 reserved */
> -#define PCI_ROM_ADDRESS_ENABLE 0x01
> -#define PCI_IO_BASE_UPPER16 0x30/* Upper half of I/O addresses */
> -#define PCI_IO_LIMIT_UPPER160x32
> -#define PCI_CAPABILITY_LIST 0x34/* Offset of first capability list
> entry */
> -#define PCI_ROM_ADDRESS1 0x38/* Same as PCI_ROM_ADDRESS, but for
> htype 1 */
> -#define PCI_INTERRUPT_LINE 0x3c/* 8 bits */
> -#define PCI_INTERRUPT_PIN0x3d/* 8 bits */
> -#define PCI_MIN_GNT 0x3e/* 8 bits */
> -#define PCI_BRIDGE_CONTROL 0x3e
> -#define PCI_MAX_LAT 0x3f/* 8 bits */
> -
> -/* Capability lists */
> -#define PCI_CAP_LIST_ID 0 /* Capability ID */
> -#define PCI_CAP_LIST_NEXT1 /* Next capability in the list */
> -
> -#define PCI_SUBSYSTEM_VENDOR_ID 0x2c
> -
> -/* Bits in the PCI Status Register (PCI 2.3 spec) */
> -#define PCI_STATUS_RESERVED1 0x007
> -#define PCI_STATUS_INT_STATUS0x008
> -#define PCI_STATUS_CAP_LIST 0x010
> -#define PCI_STATUS_66MHZ 0x020
> -#define PCI_STATUS_RESERVED2 0x040
> -#define PCI_STATUS_FAST_BACK 0x080
> -#define PCI_STATUS_DEVSEL0x600
>
> #define PCI_STATUS_RESERVED_MASK_LO (PCI_STATUS_RESERVED1 | \
> PCI_STATUS_INT_STATUS | PCI_STATUS_CAPABILITIES | \
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 08/11] acpi: use range helper function.
On Mon, Dec 14, 2009 at 09:48:23PM +0900, Isaku Yamahata wrote:
> use range helper function in pm_write_config().
>
> Signed-off-by: Isaku Yamahata
Acked-by: Michael S. Tsirkin
> ---
> hw/acpi.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index 9a69e7d..ad72297 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -437,7 +437,7 @@ static void pm_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int len)
> {
> pci_default_write_config(d, address, val, len);
> -if (address == 0x80)
> +if (range_covers_byte(address, len, 0x80))
> pm_io_space_update((PIIX4PMState *)d);
> }
>
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 09/11] piix_pci: define symbolic value for PAM0, PAM6 and SMRAM.
On Mon, Dec 14, 2009 at 09:48:24PM +0900, Isaku Yamahata wrote:
> Define symbolic value in i440fx configuration space
> for 0x59, 0x5f and 0x7f and use them.
>
> Signed-off-by: Isaku Yamahata
Good overall.
Can you pls verify that applying this patch generates
same binary as before? Small mistakes are hard to catch.
Also I am not familiar with the hardware.
Anyone on list can review this patch?
When can one get the spec for this hardware?
> ---
> hw/piix_pci.c | 13 +
> 1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 1b67475..7bbaf50 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -44,6 +44,10 @@ struct PCII440FXState {
> PIIX3State *piix3;
> };
>
> +#define I440FX_PAM0 0x59
> +#define I440FX_PAM6 0x5f
Hmm, will we end up with 7 of these?
Maybe
#define I440FX_PAM (0x59) and use
I440FX_PAM + 6 instead of I440FX_PAM6?
> +#define I440FX_SMRAM0x72
> +
> static void piix3_set_irq(void *opaque, int irq_num, int level);
>
> /* return the global irq number corresponding to a given device irq
> @@ -88,12 +92,12 @@ static void i440fx_update_memory_mappings(PCII440FXState
> *d)
> int i, r;
> uint32_t smram, addr;
>
> -update_pam(d, 0xf, 0x10, (d->dev.config[0x59] >> 4) & 3);
> +update_pam(d, 0xf, 0x10, (d->dev.config[I440FX_PAM0] >> 4) & 3);
> for(i = 0; i < 12; i++) {
> r = (d->dev.config[(i >> 1) + 0x5a] >> ((i & 1) * 4)) & 3;
is 0x5a a PAM register as well?
> update_pam(d, 0xc + 0x4000 * i, 0xc + 0x4000 * (i + 1), r);
> }
> -smram = d->dev.config[0x72];
> +smram = d->dev.config[I440FX_SMRAM];
> if ((d->smm_enabled && (smram & 0x08)) || (smram & 0x40)) {
> cpu_register_physical_memory(0xa, 0x2, 0xa);
> } else {
> @@ -132,7 +136,8 @@ static void i440fx_write_config(PCIDevice *dev,
>
> /* XXX: implement SMRAM.D_LOCK */
> pci_default_write_config(dev, address, val, len);
> -if ((address >= 0x59 && address <= 0x5f) || address == 0x72)
> +if ((address >= I440FX_PAM0 && address <= I440FX_PAM6) ||
> +address == I440FX_SMRAM)
> i440fx_update_memory_mappings(d);
> }
>
> @@ -196,7 +201,7 @@ static int i440fx_initfn(PCIDevice *dev)
> pci_config_set_class(d->dev.config, PCI_CLASS_BRIDGE_HOST);
> d->dev.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type
>
> -d->dev.config[0x72] = 0x02; /* SMRAM */
> +d->dev.config[I440FX_SMRAM] = 0x02; /* SMRAM */
I think you can remove the comment.
>
> return 0;
> }
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 10/11] piix_pci: use range helper function
On Mon, Dec 14, 2009 at 09:48:25PM +0900, Isaku Yamahata wrote:
> use range helper function in i440fx_write_config().
>
> Signed-off-by: Isaku Yamahata
> ---
> hw/piix_pci.c |6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 7bbaf50..304f84a 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -136,9 +136,11 @@ static void i440fx_write_config(PCIDevice *dev,
>
> /* XXX: implement SMRAM.D_LOCK */
> pci_default_write_config(dev, address, val, len);
> -if ((address >= I440FX_PAM0 && address <= I440FX_PAM6) ||
> -address == I440FX_SMRAM)
> +if (ranges_overlap(address, len,
> + I440FX_PAM0, I440FX_PAM6 - I440FX_PAM0 + 1) ||
So here
#define I440FX_PAM_SIZE 7
will be helpful.
> +range_covers_byte(address, len, I440FX_SMRAM)) {
> i440fx_update_memory_mappings(d);
> +}
> }
>
> static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 07/11] gt64xxx: remove gt64120_{read, write}_config().
On Mon, Dec 14, 2009 at 09:48:22PM +0900, Isaku Yamahata wrote:
> They call only pci_default_{read, write}_config().
> So they aren't necessary.
>
> Signed-off-by: Isaku Yamahata
Acked-by: Michael S. Tsirkin
> ---
> hw/gt64xxx.c | 13 +
> 1 files changed, 1 insertions(+), 12 deletions(-)
>
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index fb7f5bd..c8034e2 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -1082,17 +1082,6 @@ static void gt64120_reset(void *opaque)
> gt64120_pci_mapping(s);
> }
>
> -static uint32_t gt64120_read_config(PCIDevice *d, uint32_t address, int len)
> -{
> -return pci_default_read_config(d, address, len);
> -}
> -
> -static void gt64120_write_config(PCIDevice *d, uint32_t address, uint32_t
> val,
> - int len)
> -{
> -pci_default_write_config(d, address, val, len);
> -}
> -
> static void gt64120_save(QEMUFile* f, void *opaque)
> {
> PCIDevice *d = opaque;
> @@ -1125,7 +1114,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
> pic, 144, 4);
> s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
> d = pci_register_device(s->pci->bus, "GT64120 PCI Bus",
> sizeof(PCIDevice),
> -0, gt64120_read_config, gt64120_write_config);
> +0, NULL, NULL);
>
> /* FIXME: Malta specific hw assumptions ahead */
>
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 01/11] pci: remove PCIBus::config_reg.
On Mon, Dec 14, 2009 at 09:48:16PM +0900, Isaku Yamahata wrote:
> PCIBus::config_reg isn't used anymore, so remove it.
>
> Signed-off-by: Isaku Yamahata
Acked-by: Michael S. Tsirkin
> ---
> hw/pci.c |1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 404eead..aed3a24 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -40,7 +40,6 @@ struct PCIBus {
> pci_set_irq_fn set_irq;
> pci_map_irq_fn map_irq;
> pci_hotplug_fn hotplug;
> -uint32_t config_reg; /* XXX: suppress */
> void *irq_opaque;
> PCIDevice *devices[256];
> PCIDevice *parent_dev;
> --
> 1.6.5.4
[Qemu-devel] Re: [PATCH 04/11] pci: remove PCI_REVISION and PCI_SUBDEVICE_ID.
On Mon, Dec 14, 2009 at 09:48:19PM +0900, Isaku Yamahata wrote:
> There is no user and they're obsolete. So remove them.
>
> Signed-off-by: Isaku Yamahata
We are removing everything here later, is it worth it
to put in a separate patch?
Anyway:
Acked-by: Michael S. Tsirkin
> ---
> hw/pci.h |2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 0309674..91f3809 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -154,9 +154,7 @@ typedef struct PCIIORegion {
> #define PCI_CAP_LIST_ID 0 /* Capability ID */
> #define PCI_CAP_LIST_NEXT1 /* Next capability in the list */
>
> -#define PCI_REVISION0x08/* obsolete, use PCI_REVISION_ID */
> #define PCI_SUBSYSTEM_VENDOR_ID 0x2c
> -#define PCI_SUBDEVICE_ID0x2e/* obsolete, use PCI_SUBSYSTEM_ID */
>
> /* Bits in the PCI Status Register (PCI 2.3 spec) */
> #define PCI_STATUS_RESERVED1 0x007
> --
> 1.6.5.4
Re: [Qemu-devel] Spice project is now open
Hi, Well, in fact VNC would wait for the refresh timer of the VGA framebuffer dirty thing and only send a single update too. Well, it isn't that simple. When copyrect is used updates can be *much* more frequently. Reason is that the vnc server has to push out outstanding dirty regions before sending the copyrect command. Otherwise the client-side blit would work with stale data. cheers, Gerd
[Qemu-devel] Re: [PATCH 00/11] various pci clean ups.
On Mon, Dec 14, 2009 at 09:48:15PM +0900, Isaku Yamahata wrote:
> This patch series is for various somewhat atrandom clean up.
> Michael, this patch series possibly conflicts with your cleanups.
> Which patch should I rebase to?
My tree is here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git pci
Note: it is rebased now and then. If this becomes
a problem, let me know and I will stop.
> Isaku Yamahata (11):
> pci: remove PCIBus::config_reg.
> pci: clean up pci_bar_address()
> pci: s/PCI_SUBVENDOR_ID/PCI_SUBSYSTEM_VENDOR_ID/g
> pci: remove PCI_REVISION and PCI_SUBDEVICE_ID.
> pci: import Linux pci_regs.h
> pci: use pci_regs.h
> gt64xxx: remove gt64120_{read, write}_config().
> acpi: use range helper function.
> piix_pci: define symbolic value for PAM0, PAM6 and SMRAM.
> piix_pci: use range helper function
> msix: use range helper function.
>
> hw/acpi.c |2 +-
> hw/gt64xxx.c | 13 +-
> hw/msix.c |2 +-
> hw/pci.c |6 +-
> hw/pci.h | 74 +--
> hw/pci_regs.h | 665
> +
> hw/piix_pci.c | 15 +-
> 7 files changed, 684 insertions(+), 93 deletions(-)
> create mode 100644 hw/pci_regs.h
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On Mon, Dec 14, 2009 at 02:30:19PM +0100, Markus Armbruster wrote:
> Gerd Hoffmann writes:
>
> > On 12/14/09 12:10, Michael S. Tsirkin wrote:
> >> On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
> >> for block:
> >> if (strcmp(s->serial_str, "0"))
> >> features |= 1<< VIRTIO_BLK_F_IDENTIFY;
> >>
> >> if (bdrv_is_read_only(s->bs))
> >> features |= 1<< VIRTIO_BLK_F_RO;
> >
> > Sure you want these be configurable?
> >
> >> Also, I'd like these things to be saved in bits and not add a ton
> >> of fields in device. Ideas how to do this?
> >
> > I guess you only want disable features?
It's not an easy quesiton.
If we do it as disable bits, then we get incompatible
machines when running on different hosts.
Would it be better to fail instead?
> > You could have a bitmap property then, which accepts names for the
> > bits. It would need a table like this ...
> >
> >char *bitnames[] = {
> > [ VIRTIO_BLK_F_IDENTIFY ] = "blk-identify",
> > [ VIRTIO_BLK_F_RO [ = "blk-ro",
> > [ ... ]
> >};
> >
> > Then the property parser would accepts strings such as 'bit1|bit2' and
> > you can have
> >
> > -device 'virtio-blk-pci,disable=blk-identify|ring-indirect'
> >
> > The driver will just do 'vdev->host_features &= ~disable'.
>
> Use of '|' in option argument syntax is user-hostile, because it
> requires quoting in the shell. What about '+'?
I am guessing that '|' parsing is already there,
but yes + would be a nice touch.
--
MST
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 02:35:31PM +0100, Alexander Graf wrote: > Michael S. Tsirkin wrote: > > On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > > > >> Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > >> > >> > >>> On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > >>> > On 12/14/09 10:44, Michael S. Tsirkin wrote: > > > No, it did not even start booting the kernel. Just gave me blank > > screen. > > > [ testing ] > > Oh. That is something completely different. A bug in the rom > loader. > It fails to fit both e1000 (default nic) and virtio-net boot roms > into > the option rom area and bails out (before loading seabios). vl.c > doesn't check the return value and happily continues (without bios). > Which doesn't work out very well ... > > With two identical nics the (single) rom fits and qemu boots. > > Hmm. Of course vl.c must be fixed to check the return value. > > >>> Yes. > >>> > >>> > Not sure how to deal with the rom size issue. The gPXE roms look > quit > big compared to the older roms we had. > > >>> Hmm, it's a regression then ... > >>> > >> How does real hw handle this? I'm pretty sure most servers these days > >> use more option rom space than this. They usually have some onboard raid > >> bios, external storage, on-board nic, pci nic, ... > >> > > > > Real hardware might do several things I know about > > - option rom is typically small. > > - option rom is not loaded always (BIOS option), or not for all cards. > > There are might be other tricks. > > > > There are probably other tricks. I was booting up a machine that had > like 5 options roms going through their initialization that all weren't > exactly small. This might boil down to better ROMs. E.g. maybe they request memory with PMM and then give it up after initialization? > >> So there must be some way to just have more option rom space. > >> > > > > What do you mean? > > > > Well, what's keeping us from having 5 MB of option roms? > > >> Implementing anything else would just be a waste of time. It'd break > >> again when ppl do device assignment. > >> > >> Alex > >> > > > > We need some solution for 0.12 though IMO. > > This does not need to address device assignment, > > but it must be simple. > > > > Agreed. If there is a solution that gives us the chance to support an > arbitrary number of option roms that wouldn't take forever to implement, > I'd rather take that one though. > > Alex
[Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU
The QEMU team is pleased to announce the availability of the 0.12.0-rc2 release. This is the second release candidate for the 0.12.0 release. This release is not intended for production use. Testing release candidates is a great way to contribute to QEMU and improve the quality of the 0.12.0 release. The current plan is to release 0.12.0 on Friday, Dec 18th. Depending on the severity of bug fixes we get between now and then, we may do another release candidate in the middle of the week. For this release candidate, we would appreciate as much testing and feedback as possible, particularly the following features which have seen a lot of activity since the 0.11.0 release: - live migration; especially with non-x86 and complex machine configurations - block device migration - exotic x86 guests that may interact with the BIOS in special ways - PXE booting - SCSI disk support - any tools that depend on parsing monitor output It can be downloaded from Savannah at: http://download.savannah.gnu.org/releases/qemu/qemu-0.12.0-rc2.tar.gz Please send testing feedback (positive or negative) to qemu-devel and file bugs against the release candidate at: https://bugs.launchpad.net/qemu A detailed change log since 0.12.0-rc1 is included below. On behalf of the QEMU team, I'd like to thank everyone who contributed to make this release happen! version 0.12.0-rc2: - v2: properly save kvm system time msr registers (Glauber Costa) - convert more monitor commands to qmp (Luiz Capitulino) - vnc: fix capslock tracking logic. (Gerd Hoffmann) - QemuOpts: allow larger option values. (Gerd Hoffmann) - scsi: fix drive hotplug. (Gerd Hoffmann) - pci: don't hw_error() when no slot is available. (Gerd Hoffmann) - pci: don't abort() when trying to hotplug with acpi off. (Gerd Hoffmann) - allow default devices to be implemented in config file (Gerd Hoffman) - vc: colorize chardev title line with blue background. (Gerd Hoffmann) - chardev: make chardevs specified in config file work. (Gerd Hoffmann) - qdev: also match bus name for global properties (Gerd Hoffmann) - qdev: add command line option to set global defaults for properties. (Gerd H offmann) - kvm: x86: Save/restore exception_index (Jan Kiszka) - qdev: Replace device names containing whitespace (Markus Armbruster) - fix rtc-td-hack on host without high-res timers (Gleb Natapov) - virtio: verify features on load (Michael S. Tsirkin) - vmware_vga: add rom file so that it boots. (Dave Airlie) - Do not abort on qemu_malloc(0) in production builds (Anthony Liguori) - Fix ARM userspace strex implementation. (Paul Brook) - qemu: delete rule target on error (Michael S. Tsirkin) - QMP: add human-readable description to error response (Markus Armbruster) - convert more monitor commands to QError (Markus Armbruster) - monitor: Fix double-prompt after "change vnc passwd BLA" (Markus Armbruster) - monitor: do_cont(): Don't ask for passwords (Luiz Capitulino) - monitor: Introduce 'block_passwd' command (Luiz Capitulino) - pci: interrupt disable bit support (Michael S. Tsirkin) - pci: interrupt status bit implementation (Michael S. Tsirkin) - pci: prepare irq code for interrupt state (Michael S. Tsirkin) - msix: function mask support (Michael S. Tsirkin) - msix: macro rename for function mask support (Michael S. Tsirkin) - cpuid: Fix multicore setup on Intel (Andre Przywara) - kvm: x86: Fix initial kvm_has_msr_star (Jan Kiszka) - Update OpenBIOS images to r640 (Aurelien Jarno) -- Regards, Anthony Liguori
Re: [Qemu-devel] X support for QXL and SPICE
On 12/12/09 00:58, Soeren Sandmann wrote: Even this simple support provides a better user experience than VNC because scrolling is accelerated and doesn't result in a huge bitmap getting sent across the wire. (I don't know if VNC has support for bltblt, but even if it does, a screenscraping VNC server can't easily make use of it). cirrus vga bitblit op is mapped to vnc copyrect. X + cirrus driver + xterm make use of it for text scrolling. cheers, Gerd
Re: [Qemu-devel] Re: qdev property bug?
The old behavior with two different nic types and -boot n was "undefined". The old etherboot roms were quite large. To large to fit more than one (certainly not two). How does real hw handle this? I'm pretty sure most servers these days use more option rom space than this. They usually have some onboard raid bios, external storage, on-board nic, pci nic, ... You can disable rom loading for individual cards. So there must be some way to just have more option rom space. Implementing anything else would just be a waste of time. It'd break again when ppl do device assignment. gPXE is freakishly large as far as option roms go :-) Regards, Anthony Liguori
Re: [Qemu-devel] Re: qdev property bug?
Alexander Graf wrote: Michael S. Tsirkin wrote: On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: On 12/14/09 10:44, Michael S. Tsirkin wrote: No, it did not even start booting the kernel. Just gave me blank screen. [ testing ] Oh. That is something completely different. A bug in the rom loader. It fails to fit both e1000 (default nic) and virtio-net boot roms into the option rom area and bails out (before loading seabios). vl.c doesn't check the return value and happily continues (without bios). Which doesn't work out very well ... With two identical nics the (single) rom fits and qemu boots. Hmm. Of course vl.c must be fixed to check the return value. Yes. Not sure how to deal with the rom size issue. The gPXE roms look quit big compared to the older roms we had. Hmm, it's a regression then ... How does real hw handle this? I'm pretty sure most servers these days use more option rom space than this. They usually have some onboard raid bios, external storage, on-board nic, pci nic, ... Real hardware might do several things I know about - option rom is typically small. - option rom is not loaded always (BIOS option), or not for all cards. There are might be other tricks. There are probably other tricks. I was booting up a machine that had like 5 options roms going through their initialization that all weren't exactly small. So there must be some way to just have more option rom space. What do you mean? Well, what's keeping us from having 5 MB of option roms? For starters, option roms run in real mode when you only have 1MB of addressable memory :-) Implementing anything else would just be a waste of time. It'd break again when ppl do device assignment. Alex We need some solution for 0.12 though IMO. This does not need to address device assignment, but it must be simple. Agreed. If there is a solution that gives us the chance to support an arbitrary number of option roms that wouldn't take forever to implement, I'd rather take that one though. For 0.12, we just need to fail gracefully (meaning stop loading option roms when we run out of room). It's not a regression compared to 0.11. Regards, Anthony Liguori
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 08:11:59AM -0600, Anthony Liguori wrote: > Alexander Graf wrote: >> Michael S. Tsirkin wrote: >> >>> On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: >>> Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > >> On 12/14/09 10:44, Michael S. Tsirkin wrote: >> >>> No, it did not even start booting the kernel. Just gave me >>> blank screen. >>> >> [ testing ] >> >> Oh. That is something completely different. A bug in the rom >> loader. >> It fails to fit both e1000 (default nic) and virtio-net boot >> roms into >> the option rom area and bails out (before loading seabios). vl.c >> doesn't check the return value and happily continues (without bios). >> Which doesn't work out very well ... >> >> With two identical nics the (single) rom fits and qemu boots. >> >> Hmm. Of course vl.c must be fixed to check the return value. >> > Yes. > > >> Not sure how to deal with the rom size issue. The gPXE roms >> look quit >> big compared to the older roms we had. >> > Hmm, it's a regression then ... > How does real hw handle this? I'm pretty sure most servers these days use more option rom space than this. They usually have some onboard raid bios, external storage, on-board nic, pci nic, ... >>> Real hardware might do several things I know about >>> - option rom is typically small. >>> - option rom is not loaded always (BIOS option), or not for all cards. >>> There are might be other tricks. >>> >> >> There are probably other tricks. I was booting up a machine that had >> like 5 options roms going through their initialization that all weren't >> exactly small. >> >> So there must be some way to just have more option rom space. >>> What do you mean? >>> >> >> Well, what's keeping us from having 5 MB of option roms? >> > > For starters, option roms run in real mode when you only have 1MB of > addressable memory :-) > Implementing anything else would just be a waste of time. It'd break again when ppl do device assignment. Alex >>> We need some solution for 0.12 though IMO. >>> This does not need to address device assignment, >>> but it must be simple. >>> >> >> Agreed. If there is a solution that gives us the chance to support an >> arbitrary number of option roms that wouldn't take forever to implement, >> I'd rather take that one though. >> > > For 0.12, we just need to fail gracefully (meaning stop loading option > roms when we run out of room). It's not a regression compared to 0.11. > > Regards, > > Anthony Liguori Well I am pretty sure that I used virtio + e1000 with 0.11 and apparently I can't now. So it does look like a regression to me ... -- MST
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 04:11:43PM +0200, Michael S. Tsirkin wrote: > On Mon, Dec 14, 2009 at 08:11:59AM -0600, Anthony Liguori wrote: > > Alexander Graf wrote: > >> Michael S. Tsirkin wrote: > >> > >>> On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > >>> > Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > > > > On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > > > >> On 12/14/09 10:44, Michael S. Tsirkin wrote: > >> > >>> No, it did not even start booting the kernel. Just gave me > >>> blank screen. > >>> > >> [ testing ] > >> > >> Oh. That is something completely different. A bug in the rom > >> loader. > >> It fails to fit both e1000 (default nic) and virtio-net boot > >> roms into > >> the option rom area and bails out (before loading seabios). vl.c > >> doesn't check the return value and happily continues (without bios). > >> Which doesn't work out very well ... > >> > >> With two identical nics the (single) rom fits and qemu boots. > >> > >> Hmm. Of course vl.c must be fixed to check the return value. > >> > > Yes. > > > > > >> Not sure how to deal with the rom size issue. The gPXE roms > >> look quit > >> big compared to the older roms we had. > >> > > Hmm, it's a regression then ... > > > How does real hw handle this? I'm pretty sure most servers these > days use more option rom space than this. They usually have some > onboard raid bios, external storage, on-board nic, pci nic, ... > > >>> Real hardware might do several things I know about > >>> - option rom is typically small. > >>> - option rom is not loaded always (BIOS option), or not for all cards. > >>> There are might be other tricks. > >>> > >> > >> There are probably other tricks. I was booting up a machine that had > >> like 5 options roms going through their initialization that all weren't > >> exactly small. > >> > >> > So there must be some way to just have more option rom space. > > >>> What do you mean? > >>> > >> > >> Well, what's keeping us from having 5 MB of option roms? > >> > > > > For starters, option roms run in real mode when you only have 1MB of > > addressable memory :-) > > > Implementing anything else would just be a waste of time. It'd > break again when ppl do device assignment. > > Alex > > >>> We need some solution for 0.12 though IMO. > >>> This does not need to address device assignment, > >>> but it must be simple. > >>> > >> > >> Agreed. If there is a solution that gives us the chance to support an > >> arbitrary number of option roms that wouldn't take forever to implement, > >> I'd rather take that one though. > >> > > > > For 0.12, we just need to fail gracefully (meaning stop loading option > > roms when we run out of room). It's not a regression compared to 0.11. > > > > Regards, > > > > Anthony Liguori > > > Well I am pretty sure that I used virtio + e1000 with 0.11 > and apparently I can't now. > So it does look like a regression to me ... > Further, we should error out when device is added. Doing this during boot is way too late, management won't be able to understand such errors and won't be able to recover. > -- > MST
Re: [Qemu-devel] Re: qdev property bug?
Michael S. Tsirkin wrote: Well I am pretty sure that I used virtio + e1000 with 0.11 and apparently I can't now. So it does look like a regression to me ... That's what I said, we should make sure that we stop loading roms when we run out of room as opposed to trampling over the bios space. But let's not try to solve the problem of fitting more roms into the space than we can. Regards, Anthony Liguori
Re: [Qemu-devel] Re: qdev property bug?
Michael S. Tsirkin wrote: Further, we should error out when device is added. Doing this during boot is way too late, management won't be able to understand such errors and won't be able to recover. I don't quite understand this. In 0.11, we never loaded option roms unless a user specified -boot n. If a user specified -boot n and used more than one nic type, I'm fairly certain it would error out during start up because it would run out of option rom space. Maybe it required three types of nics, but the point still remains. In 0.12, we always load the option rom for a PCI device. An easy solution here would be to just gracefully handle the case where we ran out of option rom space and (silently) stop loading additional roms. With respect to -boot n, it makes the behavior buggy (you cannot boot from the second nic) but my original point is that that is not a regression from 0.11. For 0.13, we should probably allow a user to suppress option rom loading for a given PCI device. The limited space is a pretty good justification for that. Regards, Anthony Liguori -- MST
Re: [Qemu-devel] vmware vga + kvm interaction
Dave Airlie wrote: I actually reinvented at least one of the patches locally and it didn't seem to help, but I'll try and take a closer look today, http://repo.or.cz/w/qemu/aliguori-queue.git vmware-vga-for-dave Is the local branch I have for vmware-vga work. I'm not sure why I never pushed those patches, but I suspect it's because I was still tracking down a bug. IIRC, this branch fixes things with -enable-kvm, but I'd usually see a SEGV in qemu about 1-2 minutes after getting into X. I don't think that has anything to do with kvm though. Regards, Anthony Liguori Dave.
Re: [Qemu-devel] Spice project is now open
Gerd Hoffmann wrote: Hi, Well, in fact VNC would wait for the refresh timer of the VGA framebuffer dirty thing and only send a single update too. Well, it isn't that simple. When copyrect is used updates can be *much* more frequently. Reason is that the vnc server has to push out outstanding dirty regions before sending the copyrect command. Otherwise the client-side blit would work with stale data. Correct. It's possible to do dependency tracking in order to queue the copyrects along with the intermediate updates but so far, this hasn't seemed to be necessary. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Check rom_load_all() return value.
Gerd Hoffmann wrote:
... otherwise we'll continue without the bios loaded in case the
option roms don't fit.
Signed-off-by: Gerd Hoffmann
---
vl.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index c0d98f5..1682808 100644
--- a/vl.c
+++ b/vl.c
@@ -6031,7 +6031,10 @@ int main(int argc, char **argv, char **envp)
qdev_machine_creation_done();
-rom_load_all();
+if (rom_load_all() != 0) {
+fprintf(stderr, "rom loading failed\n");
+exit(1);
+}
Failing to load option roms shouldn't be treated as a critical failure.
The BIOS has dedicated space so no amount of option roms should prevent
the BIOS from loading.
So I think we may need to tweak rom_load_all() a bit to be more
intelligent about handling this.
Regards,
Anthony Liguori
Re: [Qemu-devel] Re: Spice project is now open
On 12/12/09 23:35, Dor Laor wrote: 2. VDI (Virtual Desktop Interface) http://www.spice-space.org/vdi.html It's an abstraction layer for graphics/keyboard/mouse/sound /usb/serial. We need it anyway regardless of spice. What is our user like to switch from vnc to SDL on runtime? It's good for usb-over-ip for remoting, for various mouse modes, etc. What does spice need which isn't present in qemu today? You can start qemu with both sdl and vnc enabled. Graphic output is sent to both. Keyboard and mouse input is accepted from both. Sound via vnc works too. What doesn't work: usb + chardevs (i.e. serial console), so we need some interface here. For graphics spice probably needs a different interface than the existing for a stupid framebuffer with copyrect op. I don't see a need for new keyboard/mouse/sound interfaces though. cheers, Gerd
Re: [Qemu-devel] Re: Spice project is now open
Avi Kivity wrote: On 12/13/2009 01:46 AM, Anthony Liguori wrote: Dan Berrange and I have been talking about being able to move VNC server into a central process such that all of the VMs can have a single VNC port that can be connected to. This greatly simplifies the firewalling logic that an administrator has to deal with. That's a problem I've already had to deal with for our management tools. We use a private network for management and we bridge the VNC traffic into the customers network so they can see the VGA session. But since that traffic can be a large range of ports and we have to tunnel the traffic through a central server to get into the customer network, it's very difficult to setup without opening up a mess of ports. I think we're currently opening a few thousand just for VNC. Seems to me the best way to handle this is to run an accept() in a server and hand the resulting fd to the vnc server in qemu using ... wait for it ... SCM_RIGHTS. I'm just happy every time someone lobs a question into the air that can be answered using SCM_RIGHTS. That's actually a great idea made even better by the use of SCM_RIGHTS :-) I think it's a bit trickier though because ideally you would want to use the vnc protocol to negotiate which vm you're connecting to. That implies that you actually need to hand over the fd in a setup state. It's complicated by any encryption protocol too. Regards, Anthony Liguori
Re: [Qemu-devel] approaches to 3D virtualisation
On Sun, 13 Dec 2009, Mark Williamson wrote: > There may be an option c) in existence. The OpenTC project (EU funded) > included a secure UI project and part of that involved a prototype > paravirtualised 3D driver. It was done primarily by a research assistant at > Cambridge University Computer Lab and basically added a paravirtual GPU > device > that essentially proxied Gallium-level commands across a shared memory > transport. > > There's a short paper on the work here, it was implemented for Xen but I > doubt > it's that Xen-specific: > http://www.xen.org/files/xensummit_oracle09/xensummit_chris.pdf > > I think there's a longer paper that might be available somewhere. > > I assume this is similar to the VMware approach, however as implemented it > did > require a Gallium 3D stack available on the host also in order to actually > render the stuff. ISTR the guy who worked on it mentioning that VMware may > have implemented a Gallium->high level 3D API translator on the host side > instead, since most hosts aren't using Gallium drivers yet. > > The main reason I mention this is that I *thought* the code was meant to be > open sourced, in which case it might at least be an interesting starting > point. I'm not quite sure where it *is* though! > Yes, the code is available somewhere and might be a good starting point; I can retrieve it if you guys are really interested. However you should be aware that the Windows drivers are completely missing in this case too. I think this discussion is really interesting and it should be split it in two parts: the VGPU interface with 3d support, and 3d remoting. Both are difficult topics to discuss and both can (should?) be used independently from one another: a VGPU infrastructure can be used to render guest 3d commands on the host (see the VMware paper); 3d remoting can be used on native too. I am curious about the VMware MKS thread mentioned in the paper, that is the backend that receives rendering requests from the guest and executes them on the host: is that available somewhere? If it is closed source and cannot be reused for Qemu\KVM\Xen, we might as well design our own VGPU interface as we like.
Re: [Qemu-devel] Re: Spice project is now open
Gerd Hoffmann wrote: On 12/12/09 23:35, Dor Laor wrote: 2. VDI (Virtual Desktop Interface) http://www.spice-space.org/vdi.html It's an abstraction layer for graphics/keyboard/mouse/sound /usb/serial. We need it anyway regardless of spice. What is our user like to switch from vnc to SDL on runtime? It's good for usb-over-ip for remoting, for various mouse modes, etc. What does spice need which isn't present in qemu today? You can start qemu with both sdl and vnc enabled. Graphic output is sent to both. Keyboard and mouse input is accepted from both. Sound via vnc works too. What doesn't work: usb + chardevs (i.e. serial console), so we need some interface here. For graphics spice probably needs a different interface than the existing for a stupid framebuffer with copyrect op. I don't see a need for new keyboard/mouse/sound interfaces though. (Looking at spice-space.org), VDI seems to be a whole lot more than just a graphics thing. It looks a plugin interface for qemu as it incorporates things like fd registration, timer registration, etc. I don't think it's really needed verses just integrating spice support directly. I don't quite understand the relationship between spice and vdi though. If libspice implements a VDI interface directly and that's what it expects people to write against verses vdi existing solely for interaction with qemu. If the later is the case, then it's probably an unnecessary abstraction. If the former is the case, then it's just the api we have to live with. Of course, you could use something like VDI to separate out the vnc server into a more well defined module. However, I'd suggest separating that modularity effort from the integration of spice. Either do that starting with the vnc server, then add spice, or add spice with tight integration, and then build a common interface. The later is probably better since it gives you more clear requirements for the interface. Regards, Anthony Liguori
Re: [Qemu-devel] Re: qdev property bug?
On 12/14/09 15:10, Anthony Liguori wrote: The old behavior with two different nic types and -boot n was "undefined". The old etherboot roms were quite large. To large to fit more than one (certainly not two). Two worked with the etherboot roms. So there must be some way to just have more option rom space. Implementing anything else would just be a waste of time. It'd break again when ppl do device assignment. gPXE is freakishly large as far as option roms go :-) With gPXE only one rom fits in. cheers, Gerd
Re: [Qemu-devel] Re: Spice project is now open
On 12/14/2009 04:42 PM, Anthony Liguori wrote: I think it's a bit trickier though because ideally you would want to use the vnc protocol to negotiate which vm you're connecting to. Right, of course. If the client can no longer choose the target using its port number, it has to select it in some other way. That implies that you actually need to hand over the fd in a setup state. It's complicated by any encryption protocol too. Yes - need to pass the encryption state. Hopefully the crypto stacks support this. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: qdev property bug?
Gerd Hoffmann wrote: On 12/14/09 15:10, Anthony Liguori wrote: The old behavior with two different nic types and -boot n was "undefined". The old etherboot roms were quite large. To large to fit more than one (certainly not two). Two worked with the etherboot roms. Yes, they were 32k. Two did not work though if you were also using extboot (which was 2k). Still, etherboot was not a BEV rom so you couldn't make sense out of two of them anyway (since they hijack int19). Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/14/09 14:30, Markus Armbruster wrote: Then the property parser would accepts strings such as 'bit1|bit2' and you can have -device 'virtio-blk-pci,disable=blk-identify|ring-indirect' The driver will just do 'vdev->host_features&= ~disable'. Use of '|' in option argument syntax is user-hostile, because it requires quoting in the shell. What about '+'? I don't care that much. I've picked '|' because it is 'or' in many programming languanges and thus sort of intuitive (at least to me). Using '+' is fine with me too. cheers, Gerd
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/14/09 14:59, Michael S. Tsirkin wrote: It's not an easy quesiton. If we do it as disable bits, then we get incompatible machines when running on different hosts. In case that one host supports feature which the other doesn't and the feature isn't masked out? Well, management failure I'd say. The whole point of the compat machine types (and properties used there) is to make sure the vm's are compatible even in case the hosts run different software versions. cheers, Gerd
[Qemu-devel] [FOR 0.12 PATCH v2] Check rom_load_all() return value.
Check rom_load_all() return value.
Also don't make option rom loading failure fatal.
Signed-off-by: Gerd Hoffmann
---
hw/loader.c |2 +-
vl.c|5 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c
index 2d7a2c4..89491a2 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -669,7 +669,7 @@ int rom_load_all(void)
"addr 0x" TARGET_FMT_plx
", size 0x%zx, max 0x" TARGET_FMT_plx ")\n",
rom->name, addr, rom->romsize, rom->max);
-return -1;
+continue;
}
} else {
/* fixed address requested */
diff --git a/vl.c b/vl.c
index fd70caa..fd93a94 100644
--- a/vl.c
+++ b/vl.c
@@ -6049,7 +6049,10 @@ int main(int argc, char **argv, char **envp)
qdev_machine_creation_done();
-rom_load_all();
+if (rom_load_all() != 0) {
+fprintf(stderr, "rom loading failed\n");
+exit(1);
+}
qemu_system_reset();
if (loadvm) {
--
1.6.5.2
Re: [Qemu-devel] Re: Spice project is now open
On Mon, Dec 14, 2009 at 08:42:12AM -0600, Anthony Liguori wrote: > Avi Kivity wrote: > >On 12/13/2009 01:46 AM, Anthony Liguori wrote: > >> > >>Dan Berrange and I have been talking about being able to move VNC > >>server into a central process such that all of the VMs can have a > >>single VNC port that can be connected to. This greatly simplifies > >>the firewalling logic that an administrator has to deal with. > >>That's a problem I've already had to deal with for our management > >>tools. We use a private network for management and we bridge the VNC > >>traffic into the customers network so they can see the VGA session. > >>But since that traffic can be a large range of ports and we have to > >>tunnel the traffic through a central server to get into the customer > >>network, it's very difficult to setup without opening up a mess of > >>ports. I think we're currently opening a few thousand just for VNC. > > > >Seems to me the best way to handle this is to run an accept() in a > >server and hand the resulting fd to the vnc server in qemu using ... > >wait for it ... SCM_RIGHTS. > > > >I'm just happy every time someone lobs a question into the air that > >can be answered using SCM_RIGHTS. > > That's actually a great idea made even better by the use of SCM_RIGHTS :-) > > I think it's a bit trickier though because ideally you would want to use > the vnc protocol to negotiate which vm you're connecting to. That > implies that you actually need to hand over the fd in a setup state. > It's complicated by any encryption protocol too. The model I had in mind was for the proxy to define a VNC extension that allows the client to query what 'desktops' are available and request switching between them at any time. The list of desktop would of course be authorized per client, and strong authentication is a must for this. Any time a switch was made, the RFB protocol would return to the 'ServerInit' state. The idea is that you should not assume a homogenous environment, and you really don't want to fall down to the lowest common denominator of extensions, nor have the proxy doing re-encoding on the FB data updates. Returning to the ServerInit state allowing the client to re-negotiate the set of encodings for the new desktop, and so the proxy can be fairly stateless and while needing to understand the wire protocol, it can just pass through the actual FB update data unchanged. The combo of the an extension for switching desktops on the fly and the encryption state problem doesn't really seem to fit with passing the VNC FD over with SCM_RIGHTS. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] Re: Spice project is now open
On Mon, Dec 14, 2009 at 04:53:07PM +0200, Avi Kivity wrote: > On 12/14/2009 04:42 PM, Anthony Liguori wrote: > >I think it's a bit trickier though because ideally you would want to > >use the vnc protocol to negotiate which vm you're connecting to. > > Right, of course. If the client can no longer choose the target using > its port number, it has to select it in some other way. > > >That implies that you actually need to hand over the fd in a setup > >state. It's complicated by any encryption protocol too. > > Yes - need to pass the encryption state. Hopefully the crypto stacks > support this. There's no mechanism for this in the SASL libraries. With GNUTLS there is the ability to preserve negotiated session state from one TLS conenection and used it upon opening the next connection to fast-track the handshake phase. This doesn't allow you to pass the state for an existing connection to a new process though and have it carry on Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] Re: Spice project is now open
On 12/14/2009 05:17 PM, Daniel P. Berrange wrote: Yes - need to pass the encryption state. Hopefully the crypto stacks support this. There's no mechanism for this in the SASL libraries. With GNUTLS there is the ability to preserve negotiated session state from one TLS conenection and used it upon opening the next connection to fast-track the handshake phase. This doesn't allow you to pass the state for an existing connection to a new process though and have it carry on This sucks. But we can ask the client to reauthenticate. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: Spice project is now open
Avi Kivity wrote: On 12/14/2009 05:17 PM, Daniel P. Berrange wrote: Yes - need to pass the encryption state. Hopefully the crypto stacks support this. There's no mechanism for this in the SASL libraries. With GNUTLS there is the ability to preserve negotiated session state from one TLS conenection and used it upon opening the next connection to fast-track the handshake phase. This doesn't allow you to pass the state for an existing connection to a new process though and have it carry on This sucks. But we can ask the client to reauthenticate. Or instead of passing the socket file descriptor, pass over a socketpair and encrypt the traffic in the server. The encryption requires no knowledge of the protocol so it can be done easily enough in the server. You're already paying the cost for copying the data. Adding in one copy shouldn't be the end of the world. Regards, Anthony Liguori
Re: [Qemu-devel] Re: Spice project is now open
Daniel P. Berrange wrote: On Mon, Dec 14, 2009 at 08:42:12AM -0600, Anthony Liguori wrote: Avi Kivity wrote: On 12/13/2009 01:46 AM, Anthony Liguori wrote: Dan Berrange and I have been talking about being able to move VNC server into a central process such that all of the VMs can have a single VNC port that can be connected to. This greatly simplifies the firewalling logic that an administrator has to deal with. That's a problem I've already had to deal with for our management tools. We use a private network for management and we bridge the VNC traffic into the customers network so they can see the VGA session. But since that traffic can be a large range of ports and we have to tunnel the traffic through a central server to get into the customer network, it's very difficult to setup without opening up a mess of ports. I think we're currently opening a few thousand just for VNC. Seems to me the best way to handle this is to run an accept() in a server and hand the resulting fd to the vnc server in qemu using ... wait for it ... SCM_RIGHTS. I'm just happy every time someone lobs a question into the air that can be answered using SCM_RIGHTS. That's actually a great idea made even better by the use of SCM_RIGHTS :-) I think it's a bit trickier though because ideally you would want to use the vnc protocol to negotiate which vm you're connecting to. That implies that you actually need to hand over the fd in a setup state. It's complicated by any encryption protocol too. The model I had in mind was for the proxy to define a VNC extension that allows the client to query what 'desktops' are available and request switching between them at any time. The list of desktop would of course be authorized per client, and strong authentication is a must for this. Any time a switch was made, the RFB protocol would return to the 'ServerInit' state. I was thinking about it a bit differently. I was envisioning the switch to be a one time thing as opposed to being able to switch back and forth. A management app can see multiple guests by connecting repeatedly to the same port. They can implement switching in the client which allows for clever things like caching state of multiple clients while not sending updates for clients that aren't actively displayed. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] A different way to ask for readonly drive
Naphtali Sprei schrieb: > Hi, > After feedback from Red Hat guys, I've decided to slightly modify the > approach to drive's readonly. > The new approach also addresses the silent fall-back to open the drives' file > as read-only when read-write fails > (permission denied) that causes unexpected behavior. > Instead of the 'readonly' boolean flag, another flag introduced (a > replacement), 'read_write' with three values [on|off|try]: > read_write=on : open with read and write permission, no fall-back to read-only > read_write=off: open with read-only permission > read_write=try: open with read and write permission and if fails, fall-back > to read-only (the default if nothing specified) > > Suggestions for better naming for flag/values welcomed. > > I've tried to explicitly pass the required flags for the bdrv_open function > in callers, but probably missed some. > > Naphtali > > ... > Instead of on/off, I'd prefer the common shortcuts rw/ro. "try" is ok, but maybe "rw-ro" is better. So here are my suggestions: read_write=rw read_write=ro read_write=rw-ro or access=rw access=ro access=rw-ro Regards, Stefan
Re: [Qemu-devel] Re: Spice project is now open
On 12/14/2009 05:10 PM, Daniel P. Berrange wrote: The model I had in mind was for the proxy to define a VNC extension that allows the client to query what 'desktops' are available and request switching between them at any time. The list of desktop would of course be authorized per client, and strong authentication is a must for this. Any time a switch was made, the RFB protocol would return to the 'ServerInit' state. The idea is that you should not assume a homogenous environment, and you really don't want to fall down to the lowest common denominator of extensions, nor have the proxy doing re-encoding on the FB data updates. Returning to the ServerInit state allowing the client to re-negotiate the set of encodings for the new desktop, and so the proxy can be fairly stateless and while needing to understand the wire protocol, it can just pass through the actual FB update data unchanged. The combo of the an extension for switching desktops on the fly and the encryption state problem doesn't really seem to fit with passing the VNC FD over with SCM_RIGHTS. You can still implement this with SCM_RIGHTS. Authenticate, select guest, drop tls, pass fd to qemu, authenticate, hack hack hack, drop tls, pass fd back to proxy, goto 10. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Qemu terminating with SIGABRT
On Sat, 12 Dec 2009 11:09:38 -0700 "David S. Ahern" wrote: > Thanks for the responses. I had forgotten that SIGABRT==abort() which > means I have to get the core file to get to the root cause. To date the > only information I have is a shell exit status of 134 which from the > bash man pages means it died due to SIGABRT. Are you using qemu-kvm or upstream qemu? Which version? Don't you have any procedure which would make the bug more likely to happen? Thanks.
Re: [Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU
2009/12/14 Anthony Liguori : > The current plan is to release 0.12.0 on Friday, Dec 18th. Depending on the > severity of bug fixes we get between now and then, we may do another release > candidate in the middle of the week. I see that the carry flag fix for sparc is included. Are you planning to include the rest of my sparc/scsi patches? They are not as drastic as the carry fix, so they shouldn't break something which isn't already broken. On the other hand, the ability to boot Solaris/sparc kernel may be important for some users. -- Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/
Re: [Qemu-devel] Qemu terminating with SIGABRT
I'm using a variant of the KVM source from RHEL5 plus a few cherry-picked patches. Host OS is RHEL 5.3. The servers are using E5540 or E5504 processors. The host OS is running from a small USB key, and there is no place to write a core file. Other accommodations have to be made to get it. Hopefully by the end of the week I can get make to reproducing the abort and capturing a core. David Ahern On 12/14/2009 09:04 AM, Luiz Capitulino wrote: > On Sat, 12 Dec 2009 11:09:38 -0700 > "David S. Ahern" wrote: > >> Thanks for the responses. I had forgotten that SIGABRT==abort() which >> means I have to get the core file to get to the root cause. To date the >> only information I have is a shell exit status of 134 which from the >> bash man pages means it died due to SIGABRT. > > Are you using qemu-kvm or upstream qemu? Which version? Don't you > have any procedure which would make the bug more likely to happen? > > Thanks.
Re: [Qemu-devel] Re: Spice project is now open
Avi Kivity wrote: You can still implement this with SCM_RIGHTS. Authenticate, select guest, drop tls, pass fd to qemu, authenticate, hack hack hack, drop tls, pass fd back to proxy, goto 10. Here's how I'd envision this working. Start qemu with: qemu -vnc proxy:/path/to/unix/domain/socket We connect to /path/to/unix/domain/socket and wait to recv file descriptors after telling the server it's name and what protocol version it supports. We treat each received file descriptor as a new connection. We use do full protocol with no specific authentication. The server runs and opens /path/to/unix/domain/socket. Whenever a client connects to the server socket, it does protocol negotiation using the least common denominator of protocol versions given it. We use a protocol extension to list and negotiate which client to connect to. Once that's been established, we send a socketpair() over the appropriate domain socket, and do appropriate negotiation to get us up to the ServerInit stage. We use a combination of DesktopResize and WMVi in the server to get the client at the appropriate state to match the ServerInit. We then (in the server) blindly proxy any data from the qemu instance to the client (encrypting it if necessary). We won't need to reencode any traffic in this model and it's pretty reasonable from a UI perspective. In fact, if we use a helper, we can probably have an even better command line for qemu. Regards, Anthony Liguori
Re: [Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU
Artyom Tarasenko wrote: 2009/12/14 Anthony Liguori : The current plan is to release 0.12.0 on Friday, Dec 18th. Depending on the severity of bug fixes we get between now and then, we may do another release candidate in the middle of the week. I see that the carry flag fix for sparc is included. Are you planning to include the rest of my sparc/scsi patches? They are not as drastic as the carry fix, so they shouldn't break something which isn't already broken. On the other hand, the ability to boot Solaris/sparc kernel may be important for some users. It's up to Blue Swirl to decide about the sparc patches. The SCSI Inquiry fix is reasonable but it missed the -rc2 deadline. I'll queue it for final release. -- Regards, Anthony Liguori
[Qemu-devel] Re: Seabios: PCI interrupt routing question
On 12/13/2009 04:12 PM, Kevin O'Connor wrote: - Forwarded message from Gleb Natapov - From: Gleb Natapov To: Kevin O'Connor Date: Sun, 13 Dec 2009 17:07:48 +0200 Subject: Re: [...@virtutech.com: [coreboot] Seabios: PCI interrupt routing question] On Thu, Dec 10, 2009 at 08:55:11AM -0500, Kevin O'Connor wrote: FYI. - Forwarded message from Magnus Christensson - From: Magnus Christensson To: coreb...@coreboot.org Date: Fri, 27 Nov 2009 09:13:02 +0100 Subject: [coreboot] Seabios: PCI interrupt routing question Hi, I have a question about the PCI INTx pin interrupt routing in Seabios. Specifically, what does the pci_slot_get_pirq function do? It looks like it assigns different interrupt numbers to devices depending on their device number. This function implement the same logic as pci_swizzle_interrupt_pin() in Linux kernel. This logic defines how PCI bridge connects INTx of each devices behind it to system board interrupt line and it is part of PCI spec (page 30 of PCI3.0 spec). Note that the function return pin, not interrupt line. To get interrupt line we look into pci_irqs[] array. The swizzling of INTx-pins happens in PCI-to-PCI bridges. But it looks like the pci_slot_get_pirq function is applied to all devices, including those on the top-level bus that are not behind any PCI-to-PCI bridge. Further, the function only looks at device (slot) and doesn't care where the device is in the PCI hierarchy. But the interrupt routing in the southbridge maps a given INTx to the same interrupt number regardless of the device number (that mapping is initialized by the code with the "activate irq remapping in PIIX" comment). To me, this looks like the INTERRUPT_LINE would be set to a value that does not match the actual interrupt routing (if (dev& 3) != 0). The code is correct if QEMU does interrupt swizzling on PIIX3 chipset level. The code in incorrect if QEMU doesn't. Swizzling like this is done by PCI-to-PCI bridges according to PCI spec, root bridge doesn't do it AFAIK, so code looks suspicious, but not because of the reason stated above. Yes, if the QEMU emulation of PIIX3 does swizzling on bus #0 (which I don't think happens in hardware), then the code would be correct (for running on QEMU). M.
[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
On 12/13/2009 04:14 PM, Kevin O'Connor wrote:
- Forwarded message from Gleb Natapov -
From: Gleb Natapov
To: Kevin O'Connor
Date: Sun, 13 Dec 2009 15:18:08 +0200
Subject: Re: [...@virtutech.com: [coreboot] [PATCH] Seabios: Fix PkgLength
calculation for the SSDT.]
On Thu, Dec 10, 2009 at 08:55:30AM -0500, Kevin O'Connor wrote:
FYI.
- Forwarded message from Magnus Christensson -
From: Magnus Christensson
To: coreb...@coreboot.org
Date: Thu, 26 Nov 2009 13:22:14 +0100
Subject: [coreboot] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
See attached patch.
M.
> From d9dc0f50b2ce756e8a3b4ede0a8ecbe76f2afcb8 Mon Sep 17 00:00:00 2001
From: Magnus Christensson
Date: Wed, 25 Nov 2009 16:26:58 +0100
Subject: [PATCH 13/13] Fix PkgLength calculation for the SSDT.
Signed-off-by: Magnus Christensson
---
src/acpi.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/acpi.c b/src/acpi.c
index 843af69..88007ae 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -464,10 +464,12 @@ build_ssdt(void)
// build processor scope header
*(ssdt_ptr++) = 0x10; // ScopeOp
if (cpu_length<= 0x3e) {
+/* Handle 1-4 CPUs with one byte encoding */
*(ssdt_ptr++) = cpu_length + 1;
} else {
-*(ssdt_ptr++) = 0x7F;
-*(ssdt_ptr++) = (cpu_length + 2)>> 6;
+/* Handle 5-314 CPUs with two byte encoding */
+*(ssdt_ptr++) = 0x40 | ((cpu_length + 1)& 0xf);
+*(ssdt_ptr++) = (cpu_length + 1)>> 4;
Should be cpu_length + 2 as far as I can tell. The current code is
definitely broken.
Right. That should be cpu_length +2 in the else-part.
M.
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 08:25:53AM -0600, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> >> Further, we should error out when device is added. >> Doing this during boot is way too late, management >> won't be able to understand such errors and >> won't be able to recover. >> > > I don't quite understand this. > > In 0.11, we never loaded option roms unless a user specified -boot n. > If a user specified -boot n and used more than one nic type, I'm fairly > certain it would error out during start up because it would run out of > option rom space. Maybe it required three types of nics, but the point > still remains. > > In 0.12, we always load the option rom for a PCI device. An easy > solution here would be to just gracefully handle the case where we ran > out of option rom space and (silently) stop loading additional roms. > With respect to -boot n, it makes the behavior buggy (you cannot boot > from the second nic) but my original point is that that is not a > regression from 0.11. Sorry, I misunderstood what you were saying. I thought you suggested handling it with exit(1) or something like that. Yes, this hack will work I think, unless we want to go back to 0.11 behaviour. > For 0.13, we should probably allow a user to suppress option rom loading > for a given PCI device. The limited space is a pretty good > justification for that. > > Regards, > > Anthony Liguori > >>> -- >>> MST >>>
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On Mon, Dec 14, 2009 at 04:01:33PM +0100, Gerd Hoffmann wrote: > On 12/14/09 14:59, Michael S. Tsirkin wrote: >> It's not an easy quesiton. >> If we do it as disable bits, then we get incompatible >> machines when running on different hosts. > > In case that one host supports feature which the other doesn't and the > feature isn't masked out? Well, management failure I'd say. The whole > point of the compat machine types (and properties used there) is to make > sure the vm's are compatible even in case the hosts run different > software versions. > > cheers, > Gerd So how do you do this? Assume we have -disable_hw_csum. We want new machine type to have it off, right? But now you run qemu on host which does not support hw_csum. With your suggestion it will not enable hw csum? -- MST
[Qemu-devel] Re: -serial stdio broken
On Mon, Dec 14, 2009 at 10:55 AM, Gerd Hoffmann wrote: > On 12/13/09 10:38, Blue Swirl wrote: >> >> On Sun, Dec 13, 2009 at 8:24 AM, Blue Swirl wrote: >>> >>> I guess e1c09175bc00dd8dfb2ad1b26e1858dcdc109b59 or >>> 998bbd74b9d813b14a3a3b5009a5d5a48c7dce51 broke -serial stdio for all >>> targets: >>> qemu -serial stdio -monitor stdio > > Oh. It is actually used on the command line. Hmm. > > First, you can use '-serial mon:stdio' instead. > > Second, with 'qemu -nographic' you don't need to specify this at all because > that is the default. > > What is gone now is the automagic conversion of '-serial stdio -monitor > stdio' into '-serial mon:stdio' because I didn't expect people actually > using that on the command line (see commit message). Also this kind of > post-processing is pretty horrible thing for the command line parser code. > Thus I would pretty much prefer to not re-introduce this ... It looks like vl.c was a poor place to do the mux choice. I just wonder why mux option can't be automatically enabled for stdio in qemu-char.c:qemu_chr_parse_compat() or somewhere nearby. > Can you live with one of the alternatives outlined above? I removed -monitor stdio from all my tests, now they work.
Re: [Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU
On Mon, Dec 14, 2009 at 4:20 PM, Anthony Liguori wrote: > Artyom Tarasenko wrote: >> >> 2009/12/14 Anthony Liguori : >> >>> >>> The current plan is to release 0.12.0 on Friday, Dec 18th. Depending on >>> the >>> severity of bug fixes we get between now and then, we may do another >>> release >>> candidate in the middle of the week. >>> >> >> I see that the carry flag fix for sparc is included. Are you planning >> to include the rest of my sparc/scsi patches? They are not as drastic >> as the carry fix, so they shouldn't break something which isn't >> already broken. On the other hand, the ability to boot Solaris/sparc >> kernel may be important for some users. >> > > It's up to Blue Swirl to decide about the sparc patches. The SCSI Inquiry > fix is reasonable but it missed the -rc2 deadline. I'll queue it for final > release. The FDC fix should be OK. Technically AFX patch adds new functionality, but very limited.
Re: [Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU
On 12/14/2009 04:06 PM, Anthony Liguori wrote: The QEMU team is pleased to announce the availability of the 0.12.0-rc2 release. This is the second release candidate for the 0.12.0 release. This release is not intended for production use. I'm missing the tag for this release? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 04:01:43PM +0200, Michael S. Tsirkin wrote: > On Mon, Dec 14, 2009 at 02:35:31PM +0100, Alexander Graf wrote: > > Michael S. Tsirkin wrote: > > > On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > > > > > >> Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > > >> > > >> > > >>> On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > > >>> > > On 12/14/09 10:44, Michael S. Tsirkin wrote: > > > > > No, it did not even start booting the kernel. Just gave me blank > > > screen. > > > > > [ testing ] > > > > Oh. That is something completely different. A bug in the rom > > loader. > > It fails to fit both e1000 (default nic) and virtio-net boot roms > > into > > the option rom area and bails out (before loading seabios). vl.c > > doesn't check the return value and happily continues (without bios). > > Which doesn't work out very well ... > > > > With two identical nics the (single) rom fits and qemu boots. > > > > Hmm. Of course vl.c must be fixed to check the return value. > > > > >>> Yes. > > >>> > > >>> > > Not sure how to deal with the rom size issue. The gPXE roms look > > quit > > big compared to the older roms we had. > > > > >>> Hmm, it's a regression then ... > > >>> > > >> How does real hw handle this? I'm pretty sure most servers these days > > >> use more option rom space than this. They usually have some onboard raid > > >> bios, external storage, on-board nic, pci nic, ... > > >> > > > > > > Real hardware might do several things I know about > > > - option rom is typically small. > > > - option rom is not loaded always (BIOS option), or not for all cards. > > > There are might be other tricks. > > > > > > > There are probably other tricks. I was booting up a machine that had > > like 5 options roms going through their initialization that all weren't > > exactly small. > > This might boil down to better ROMs. E.g. > maybe they request memory with PMM and then give it up > after initialization? That would be my guess. However, gpxe rom are also used in real hardware. Maybe there is some config trick we're missing?
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 04:11:43PM +0200, Michael S. Tsirkin wrote: > On Mon, Dec 14, 2009 at 08:11:59AM -0600, Anthony Liguori wrote: > > Alexander Graf wrote: > >> Michael S. Tsirkin wrote: > >> > >>> On Mon, Dec 14, 2009 at 12:55:28PM +0100, Alexander Graf wrote: > >>> > Am 14.12.2009 um 11:59 schrieb "Michael S. Tsirkin" : > > > > On Mon, Dec 14, 2009 at 11:16:34AM +0100, Gerd Hoffmann wrote: > > > >> On 12/14/09 10:44, Michael S. Tsirkin wrote: > >> > >>> No, it did not even start booting the kernel. Just gave me > >>> blank screen. > >>> > >> [ testing ] > >> > >> Oh. That is something completely different. A bug in the rom > >> loader. > >> It fails to fit both e1000 (default nic) and virtio-net boot > >> roms into > >> the option rom area and bails out (before loading seabios). vl.c > >> doesn't check the return value and happily continues (without bios). > >> Which doesn't work out very well ... > >> > >> With two identical nics the (single) rom fits and qemu boots. > >> > >> Hmm. Of course vl.c must be fixed to check the return value. > >> > > Yes. > > > > > >> Not sure how to deal with the rom size issue. The gPXE roms > >> look quit > >> big compared to the older roms we had. > >> > > Hmm, it's a regression then ... > > > How does real hw handle this? I'm pretty sure most servers these > days use more option rom space than this. They usually have some > onboard raid bios, external storage, on-board nic, pci nic, ... > > >>> Real hardware might do several things I know about > >>> - option rom is typically small. > >>> - option rom is not loaded always (BIOS option), or not for all cards. > >>> There are might be other tricks. > >>> > >> > >> There are probably other tricks. I was booting up a machine that had > >> like 5 options roms going through their initialization that all weren't > >> exactly small. > >> > >> > So there must be some way to just have more option rom space. > > >>> What do you mean? > >>> > >> > >> Well, what's keeping us from having 5 MB of option roms? > >> > > > > For starters, option roms run in real mode when you only have 1MB of > > addressable memory :-) > > > Implementing anything else would just be a waste of time. It'd > break again when ppl do device assignment. > > Alex > > >>> We need some solution for 0.12 though IMO. > >>> This does not need to address device assignment, > >>> but it must be simple. > >>> > >> > >> Agreed. If there is a solution that gives us the chance to support an > >> arbitrary number of option roms that wouldn't take forever to implement, > >> I'd rather take that one though. > >> > > > > For 0.12, we just need to fail gracefully (meaning stop loading option > > roms when we run out of room). It's not a regression compared to 0.11. > > > > Regards, > > > > Anthony Liguori > > > Well I am pretty sure that I used virtio + e1000 with 0.11 > and apparently I can't now. > So it does look like a regression to me ... e1000 is the problem here, since it is by far the largest roms of them all.
Re: [Qemu-devel] Re: qdev property bug?
On Mon, Dec 14, 2009 at 08:22:12AM -0600, Anthony Liguori wrote: > Michael S. Tsirkin wrote: > >Well I am pretty sure that I used virtio + e1000 with 0.11 > >and apparently I can't now. > >So it does look like a regression to me ... > > That's what I said, we should make sure that we stop loading roms > when we run out of room as opposed to trampling over the bios space. Can't we first load the bios? Then we're pretty sure oproms will never trample it.
[Qemu-devel] Re: -serial stdio broken
It looks like vl.c was a poor place to do the mux choice. I just wonder why mux option can't be automatically enabled for stdio in qemu-char.c:qemu_chr_parse_compat() or somewhere nearby. The mux driver grabs the 'Ctrl-a' hotkey, so I wouldn't enable that unconditionally because the chardev isn't transparent any more with mux enabled. It also wouldn't really help with command line parsing. cheers, Gerd
Re: [Qemu-devel] Re: [PATCH RFC] virtio: add features qdev property
On 12/14/09 17:23, Michael S. Tsirkin wrote: On Mon, Dec 14, 2009 at 04:01:33PM +0100, Gerd Hoffmann wrote: So how do you do this? Assume we have -disable_hw_csum. We want new machine type to have it off, right? But now you run qemu on host which does not support hw_csum. With your suggestion it will not enable hw csum? I have trouble getting the setup you are talking about ... Sounds like hw_csum could be enabled/disabled depending on the hardware capabilities on the host. Is that correct? cheers, Gerd
