Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState
On Fri, 2014-05-30 at 16:45 -0300, Eduardo Habkost wrote:
> On Mon, May 26, 2014 at 03:40:58PM +0300, Marcel Apfelbaum wrote:
> > Make machine's QemuOpts QOM properties of machine. The properties
> > are automatically filled in. This opens the possiblity to create
> > opts per machine rather than global.
> >
> > Signed-off-by: Marcel Apfelbaum
> > ---
> > hw/core/machine.c | 256
> >
> > include/hw/boards.h | 6 +-
> > vl.c| 10 +-
> > 3 files changed, 266 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index d3ffef7..dbcf2a1 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -11,6 +11,260 @@
> > */
> >
> > #include "hw/boards.h"
> > +#include "qapi/visitor.h"
> > +
> > +static char *machine_get_accel(Object *obj, Error **errp)
> > +{
> > +MachineState *ms = MACHINE(obj);
> > +return g_strdup(ms->accel);
> > +}
> > +
> > +static void machine_set_accel(Object *obj, const char *value, Error **errp)
> > +{
> > +MachineState *ms = MACHINE(obj);
> > +ms->accel = g_strdup(value);
> > +}
>
> You are not freeing the old value here and on the other string setters.
It seems to be the caller responsibility.
>
> (Do we have some existing common wrapper to automatically free the old
> value and set the new one, or every setter should duplicate the same
> free/strdup logic?)
We don't have yet, from what I know, and this is for the moment the QMP way
to handle string attributes.
Thanks,
Marcel
>
Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState
On Fri, 2014-05-30 at 16:25 -0300, Eduardo Habkost wrote:
> On Mon, May 26, 2014 at 03:40:58PM +0300, Marcel Apfelbaum wrote:
> [...]
> > +static void machine_initfn(Object *obj)
> > +{
> > +object_property_add_str(obj, "accel",
> > +machine_get_accel, machine_set_accel, NULL);
> > +object_property_add_bool(obj, "kernel_irqchip",
> > + machine_get_kernel_irqchip,
> > + machine_set_kernel_irqchip,
> > + NULL);
>
> In the case of kernel_irqchip, the information contained in MachineState
> is not a superset of the information contained on
> qemu_get_machine_opts().
>
> See hw/ppc/{e500,spapr}.c. They use kernel_irqchip like this:
>
> bool irqchip_allowed = qemu_opt_get_bool(machine_opts,
> "kernel_irqchip", true);
> bool irqchip_required = qemu_opt_get_bool(machine_opts,
> "kernel_irqchip", false);
>
> if (irqchip_allowed) {
> dev = ppce500_init_mpic_kvm(params, irqs);
> }
>
> if (irqchip_required && !dev) {
> fprintf(stderr, "%s: irqchip requested but unavailable\n",
> __func__);
> abort();
> }
>
> This means kernel_irqchip have three possible states: "disabled", "required",
> and "allowed".
I already had a patch adding "property_is_set" to QMP, but was not accepted
as there is no way yet to "unset" a property. (I can point you to the series)
>
> This means that MachineState.kernel_irqchip is not usable by current
> code that uses the kernel_irqchip option. I suppose we plan to address
> this on MachineState, too, to not get stuck with a global
> qemu_get_machine_opts() forever?
I completely agree with you and I already had a patch tackling it,
based on "property_is_set", but was no accepted yet, obviously.
I was instructed to set the default value in the machine init function
and some way (I don't remember now) to emulate required/allowed.
I do plan to get back to this.
Thanks,
Marcel
>
Re: [Qemu-devel] [PATCH 3/3] hw/machine: Free old values of string properties
On Fri, 2014-05-30 at 17:02 -0300, Eduardo Habkost wrote:
> Signed-off-by: Eduardo Habkost
> ---
> Cc: Marcel Apfelbaum
> Cc: Andreas Färber
> ---
> hw/core/machine.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cbba679..df612bb 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,6 +24,7 @@ static void machine_set_accel(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->accel);
I am not sure if in QMP is not caller's responsibility to free the input string.
If I think about it, I ask an object to set "my" string and it deletes it :(...
Same for the others.
Added Markus and Luiz, maybe they have an opinion on that.
Thanks,
Marcel
> ms->accel = g_strdup(value);
> }
>
> @@ -79,6 +80,7 @@ static void machine_set_kernel(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->kernel_filename);
> ms->kernel_filename = g_strdup(value);
> }
>
> @@ -93,6 +95,7 @@ static void machine_set_initrd(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->initrd_filename);
> ms->initrd_filename = g_strdup(value);
> }
>
> @@ -107,6 +110,7 @@ static void machine_set_append(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->kernel_cmdline);
> ms->kernel_cmdline = g_strdup(value);
> }
>
> @@ -121,6 +125,7 @@ static void machine_set_dtb(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dtb);
> ms->dtb = g_strdup(value);
> }
>
> @@ -135,6 +140,7 @@ static void machine_set_dumpdtb(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dumpdtb);
> ms->dumpdtb = g_strdup(value);
> }
>
> @@ -176,6 +182,7 @@ static void machine_set_dt_compatible(Object *obj, const
> char *value, Error **er
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->dt_compatible);
> ms->dt_compatible = g_strdup(value);
> }
>
> @@ -232,6 +239,7 @@ static void machine_set_firmware(Object *obj, const char
> *value, Error **errp)
> {
> MachineState *ms = MACHINE(obj);
>
> +g_free(ms->firmware);
> ms->firmware = g_strdup(value);
> }
>
Re: [Qemu-devel] [PATCH] machine: Add kvm-type property
On Fri, 2014-05-30 at 17:41 -0300, Eduardo Habkost wrote:
> The kvm-type machine option was left out when MachineState was
> introduced, preventing the kvm-type option from being used. Add the
> missing property.
Very interesting how did I miss that.
Thanks!
Marcel
>
> Signed-off-by: Eduardo Habkost
> Cc: Andreas Färber
> Cc: Aneesh Kumar K.V
> Cc: Alexander Graf
> Cc: Marcel Apfelbaum
> ---
> Tested in a x86 machine only. Help would be welcome to test it on a PPC
> machine using -machine spapr and KVM.
>
> Before this patch:
>
> $ qemu-system-x86_64 -machine pc,kvm-type=hv,accel=kvm
> qemu-system-x86_64: Property '.kvm-type' not found
>
> (This means the option won't work even for sPAPR machines.)
>
> After applying this patch:
>
> $ qemu-system-x86_64 -machine pc,kvm-type=hv,accel=kvm
> Invalid argument kvm-type=hv
>
> (This means the x86 KVM init code is seeing (and rejecting) the option,
> and the sPAPR code can use it.)
>
> Note that qemu-system-x86_64 will segfault with the above command-line
> unless an additional fix (submitted today) is applied (kvm: Ensure
> negative return value on kvm_init() error handling path).
> ---
> hw/core/machine.c | 17 +
> include/hw/boards.h | 1 +
> 2 files changed, 18 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cbba679..ed47b3a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -235,6 +235,21 @@ static void machine_set_firmware(Object *obj, const char
> *value, Error **errp)
> ms->firmware = g_strdup(value);
> }
>
> +static char *machine_get_kvm_type(Object *obj, Error **errp)
> +{
> +MachineState *ms = MACHINE(obj);
> +
> +return g_strdup(ms->kvm_type);
> +}
> +
> +static void machine_set_kvm_type(Object *obj, const char *value, Error
> **errp)
> +{
> +MachineState *ms = MACHINE(obj);
> +
> +g_free(ms->kvm_type);
> +ms->kvm_type = g_strdup(value);
> +}
> +
> static void machine_initfn(Object *obj)
> {
> object_property_add_str(obj, "accel",
> @@ -274,6 +289,8 @@ static void machine_initfn(Object *obj)
> object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb,
> NULL);
> object_property_add_str(obj, "firmware",
> machine_get_firmware, machine_set_firmware,
> NULL);
> +object_property_add_str(obj, "kvm-type",
> +machine_get_kvm_type, machine_set_kvm_type,
> NULL);
> }
>
> static void machine_finalize(Object *obj)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 2d2e2be..44956d6 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -111,6 +111,7 @@ struct MachineState {
> bool mem_merge;
> bool usb;
> char *firmware;
> +char *kvm_type;
>
> ram_addr_t ram_size;
> const char *boot_order;
Re: [Qemu-devel] [PATCH v3 1/3] e1000: allow command-line selection of card model
On Sat, May 31, 2014 at 11:33:14PM -0400, Gabriel L. Somlo wrote:
> Allow selection of different card models from the qemu
> command line, to better accomodate a wider range of guests.
>
> Signed-off-by: Romain Dolbeau
> Signed-off-by: Gabriel Somlo
> ---
> hw/net/e1000.c | 120
> +---
> hw/net/e1000_regs.h | 6 +++
> 2 files changed, 102 insertions(+), 24 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 8387443..4e208e3 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -69,23 +69,13 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
>
> /*
> * HW models:
> - * E1000_DEV_ID_82540EM works with Windows and Linux
> + * E1000_DEV_ID_82540EM works with Windows, Linux, and OS X <= 10.8
> * E1000_DEV_ID_82573L OK with windoze and Linux 2.6.22,
> * appears to perform better than 82540EM, but breaks with Linux 2.6.18
> * E1000_DEV_ID_82544GC_COPPER appears to work; not well tested
> + * E1000_DEV_ID_82545EM_COPPER works with Linux and OS X >= 10.6
> * Others never tested
> */
> -enum { E1000_DEVID = E1000_DEV_ID_82540EM };
> -
> -/*
> - * May need to specify additional MAC-to-PHY entries --
> - * Intel's Windows driver refuses to initialize unless they match
> - */
> -enum {
> -PHY_ID2_INIT = E1000_DEVID == E1000_DEV_ID_82573L ? 0xcc2 :
> - E1000_DEVID == E1000_DEV_ID_82544GC_COPPER ? 0xc30 :
> - /* default to E1000_DEV_ID_82540EM */ 0xc20
> -};
>
> typedef struct E1000State_st {
> /*< private >*/
> @@ -151,10 +141,21 @@ typedef struct E1000State_st {
> uint32_t compat_flags;
> } E1000State;
>
> -#define TYPE_E1000 "e1000"
> +typedef struct E1000DeviceClass {
> +PCIDeviceClass parent_class;
> +uint16_t phy_id2;
> +bool is_8257xx;
> +} E1000DeviceClass;
> +
> +#define TYPE_E1000_BASE "e1000-base"
>
> #define E1000(obj) \
> -OBJECT_CHECK(E1000State, (obj), TYPE_E1000)
> +OBJECT_CHECK(E1000State, (obj), TYPE_E1000_BASE)
> +
> +#define E1000_DEVICE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(E1000DeviceClass, (klass), TYPE_E1000_BASE)
> +#define E1000_DEVICE_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(E1000DeviceClass, (obj), TYPE_E1000_BASE)
>
> #define defreg(x) x = (E1000_##x>>2)
> enum {
> @@ -232,10 +233,11 @@ static const char phy_regcap[0x20] = {
> [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R
> };
>
> +/* PHY_ID2 documented in 8254x_GBe_SDM.pdf, pp. 250 */
> static const uint16_t phy_reg_init[] = {
> [PHY_CTRL] = 0x1140,
> [PHY_STATUS] = 0x794d, /* link initially up with not completed autoneg */
> -[PHY_ID1] = 0x141, [PHY_ID2] =
> PHY_ID2_INIT,
> +[PHY_ID1] = 0x141, /* [PHY_ID2] configured per DevId, from e1000_reset()
> */
> [PHY_1000T_CTRL] = 0x0e00,
> [M88E1000_PHY_SPEC_CTRL] = 0x360,
> [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, [PHY_AUTONEG_ADV] = 0xde1,
> [PHY_LP_ABILITY] = 0x1e0,[PHY_1000T_STATUS] =
> 0x3c00,
> @@ -269,13 +271,15 @@ static void
> set_interrupt_cause(E1000State *s, int index, uint32_t val)
> {
> PCIDevice *d = PCI_DEVICE(s);
> +E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
> uint32_t pending_ints;
> uint32_t mit_delay;
>
> -if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) {
> -/* Only for 8257x */
> +if (val && edc->is_8257xx) {
> +/* hack only for 8257xx models */
> val |= E1000_ICR_INT_ASSERTED;
> }
> +
> s->mac_reg[ICR] = val;
>
> /*
> @@ -375,6 +379,7 @@ rxbufsize(uint32_t v)
> static void e1000_reset(void *opaque)
> {
> E1000State *d = opaque;
> +E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
> uint8_t *macaddr = d->conf.macaddr.a;
> int i;
>
> @@ -385,6 +390,7 @@ static void e1000_reset(void *opaque)
> d->mit_ide = 0;
> memset(d->phy_reg, 0, sizeof d->phy_reg);
> memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
> +d->phy_reg[PHY_ID2] = edc->phy_id2;
> memset(d->mac_reg, 0, sizeof d->mac_reg);
> memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
> d->rxbuf_min_shift = 1;
> @@ -1440,9 +1446,13 @@ static const VMStateDescription vmstate_e1000 = {
> }
> };
>
> +/*
> + * EEPROM contents documented in Tables 5-2 and 5-3, pp. 98-102.
> + * Note: A valid DevId will be inserted during pci_e1000_init().
> + */
> static const uint16_t e1000_eeprom_template[64] = {
> 0x, 0x, 0x, 0x, 0x, 0x, 0x, 0x,
> -0x3000, 0x1000, 0x6403, E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, 0x3040,
> +0x3000, 0x1000, 0x6403, 0 /*DevId*/, 0x8086, 0 /*DevId*/, 0x8086, 0x3040,
> 0x0008, 0x2000, 0x7e14, 0x0048, 0x1000, 0x00d8, 0x, 0x2700,
> 0x6cc9, 0x3150, 0x0722, 0x040b, 0x0984, 0x, 0xc000, 0x0706,
Re: [Qemu-devel] [PATCH v3 0/3] e1000: allow model/device_id selection on command line
On Sat, May 31, 2014 at 11:33:13PM -0400, Gabriel L. Somlo wrote: > Allow selection of different card models from the qemu > command line, to better accomodate a wider range of guests. Looks good to me. If possible pls address a nit I noted in one of the patches. Besides that: Reviewed-by: Michael S. Tsirkin > New in v3: > > - 1/3 and 2/3 from v2 now merged into a single patch (1/3), with: > - s/TYPE_E1000/TYPE_E1000_BASE/ as suggested by Stefan > - improved QOM-ification as suggested by Peter Crosthwaite > > - *OPTIONAL* patch to remove stale support for 8257xx (see commit blurb > in patch 3/3 for details > (this can be squashed on top of 1/3, but I'm including it separately > here for clarity, and as an RFC). > > Thanks, > Gabriel > > > v2: > > - moved check for 8257x out of the way of QOM, as suggested by > Michael (patch 1/3) > > - resolved "Signed-off-by" misunderstanding and miscellaneous style > issues (patch 2/3) > > - modified e1000 test to check for all supported models, as suggested > by Andreas (patch 3/3). I used eepro100-test.c as an example for > this change. > > > Gabriel L. Somlo (3): > e1000: allow command-line selection of card model > tests: e1000: test additional device IDs > e1000: remove broken support for 82573L > > hw/net/e1000.c | 110 > +++- > hw/net/e1000_regs.h | 6 +++ > tests/e1000-test.c | 33 > 3 files changed, 114 insertions(+), 35 deletions(-) > > -- > 1.9.3
[Qemu-devel] KVM call agenda ideas
Hi Juan, I want to add to the call "Machine as QOM" next steps: - Solve the 'sensible' issues. - Choose as first tasks the ones that will be more useful. - Select solutions agreed by everyone. Thanks, Marcel
Re: [Qemu-devel] [PATCH v3 1/3] e1000: allow command-line selection of card model
On Sun, Jun 1, 2014 at 1:33 PM, Gabriel L. Somlo wrote:
> Allow selection of different card models from the qemu
> command line, to better accomodate a wider range of guests.
>
> Signed-off-by: Romain Dolbeau
> Signed-off-by: Gabriel Somlo
Reviewed-by: Peter Crosthwaite
> ---
> hw/net/e1000.c | 120
> +---
> hw/net/e1000_regs.h | 6 +++
> 2 files changed, 102 insertions(+), 24 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 8387443..4e208e3 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -69,23 +69,13 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
>
> /*
> * HW models:
> - * E1000_DEV_ID_82540EM works with Windows and Linux
> + * E1000_DEV_ID_82540EM works with Windows, Linux, and OS X <= 10.8
> * E1000_DEV_ID_82573L OK with windoze and Linux 2.6.22,
> * appears to perform better than 82540EM, but breaks with Linux 2.6.18
> * E1000_DEV_ID_82544GC_COPPER appears to work; not well tested
> + * E1000_DEV_ID_82545EM_COPPER works with Linux and OS X >= 10.6
> * Others never tested
> */
> -enum { E1000_DEVID = E1000_DEV_ID_82540EM };
> -
> -/*
> - * May need to specify additional MAC-to-PHY entries --
> - * Intel's Windows driver refuses to initialize unless they match
> - */
> -enum {
> -PHY_ID2_INIT = E1000_DEVID == E1000_DEV_ID_82573L ?0xcc2
> :
> - E1000_DEVID == E1000_DEV_ID_82544GC_COPPER ?0xc30
> :
> - /* default to E1000_DEV_ID_82540EM */ 0xc20
> -};
>
> typedef struct E1000State_st {
> /*< private >*/
> @@ -151,10 +141,21 @@ typedef struct E1000State_st {
> uint32_t compat_flags;
> } E1000State;
>
> -#define TYPE_E1000 "e1000"
> +typedef struct E1000DeviceClass {
> +PCIDeviceClass parent_class;
> +uint16_t phy_id2;
> +bool is_8257xx;
> +} E1000DeviceClass;
> +
> +#define TYPE_E1000_BASE "e1000-base"
>
> #define E1000(obj) \
> -OBJECT_CHECK(E1000State, (obj), TYPE_E1000)
> +OBJECT_CHECK(E1000State, (obj), TYPE_E1000_BASE)
> +
> +#define E1000_DEVICE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(E1000DeviceClass, (klass), TYPE_E1000_BASE)
> +#define E1000_DEVICE_GET_CLASS(obj) \
> +OBJECT_GET_CLASS(E1000DeviceClass, (obj), TYPE_E1000_BASE)
>
> #definedefreg(x) x = (E1000_##x>>2)
> enum {
> @@ -232,10 +233,11 @@ static const char phy_regcap[0x20] = {
> [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R
> };
>
> +/* PHY_ID2 documented in 8254x_GBe_SDM.pdf, pp. 250 */
> static const uint16_t phy_reg_init[] = {
> [PHY_CTRL] = 0x1140,
> [PHY_STATUS] = 0x794d, /* link initially up with not completed autoneg */
> -[PHY_ID1] = 0x141, [PHY_ID2] = PHY_ID2_INIT,
> +[PHY_ID1] = 0x141, /* [PHY_ID2] configured per DevId, from e1000_reset()
> */
> [PHY_1000T_CTRL] = 0x0e00, [M88E1000_PHY_SPEC_CTRL] =
> 0x360,
> [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, [PHY_AUTONEG_ADV] = 0xde1,
> [PHY_LP_ABILITY] = 0x1e0, [PHY_1000T_STATUS] = 0x3c00,
> @@ -269,13 +271,15 @@ static void
> set_interrupt_cause(E1000State *s, int index, uint32_t val)
> {
> PCIDevice *d = PCI_DEVICE(s);
> +E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
There was an earlier comment along the lines of avoiding QOM in the
data-path. Although I don't see a clean way around it TBH. Is there
any perf degredation in high interrupt scenarios?
My first guess would be no, as the class cast cache would be low
chance of evictions causing expensive lookup failures and the problem
probably evaporates #ifndef QOM_CAST_DEBUG anyways.
So I think this is ok.
Regards,
Peter
> uint32_t pending_ints;
> uint32_t mit_delay;
>
> -if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) {
> -/* Only for 8257x */
> +if (val && edc->is_8257xx) {
> +/* hack only for 8257xx models */
> val |= E1000_ICR_INT_ASSERTED;
> }
> +
> s->mac_reg[ICR] = val;
>
> /*
> @@ -375,6 +379,7 @@ rxbufsize(uint32_t v)
> static void e1000_reset(void *opaque)
> {
> E1000State *d = opaque;
> +E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
> uint8_t *macaddr = d->conf.macaddr.a;
> int i;
>
> @@ -385,6 +390,7 @@ static void e1000_reset(void *opaque)
> d->mit_ide = 0;
> memset(d->phy_reg, 0, sizeof d->phy_reg);
> memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
> +d->phy_reg[PHY_ID2] = edc->phy_id2;
> memset(d->mac_reg, 0, sizeof d->mac_reg);
> memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
> d->rxbuf_min_shift = 1;
> @@ -1440,9 +1446,13 @@ static const VMStateDescription vmstate_e1000 = {
> }
> };
>
> +/*
> + * EEPROM contents documented in Tables 5-2 and 5-3, pp. 98-102.
> + * Note: A valid DevId will be inserted during pci_e1000_init().
> + */
> static const uint16_t e1000_eeprom_template[64] =
Re: [Qemu-devel] [PATCH v3 2/3] tests: e1000: test additional device IDs
On Sun, Jun 1, 2014 at 1:33 PM, Gabriel L. Somlo wrote:
> Update e1000-test.c to check all currently supported devices.
>
> Suggested-by: Andreas Färber
> Signed-off-by: Gabriel Somlo
> Reviewed-by: Michael S. Tsirkin
Reviewed-by: Peter Crosthwaite
> ---
> tests/e1000-test.c | 34 +++---
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/tests/e1000-test.c b/tests/e1000-test.c
> index a8ba2fc..53c41f8 100644
> --- a/tests/e1000-test.c
> +++ b/tests/e1000-test.c
> @@ -13,21 +13,41 @@
> #include "qemu/osdep.h"
>
> /* Tests only initialization so far. TODO: Replace with functional tests */
> -static void nop(void)
> +static void test_device(gconstpointer data)
> {
> +const char *model = data;
> +QTestState *s;
> +char *args;
> +
> +args = g_strdup_printf("-device %s", model);
> +s = qtest_start(args);
> +
> +if (s) {
> +qtest_quit(s);
> +}
> +g_free(args);
> }
>
> +static const char *models[] = {
> +"e1000",
> +"e1000-82540em",
> +"e1000-82544gc",
> +"e1000-82545em",
> +"e1000-82573l",
> +};
> +
> int main(int argc, char **argv)
> {
> -int ret;
> +int i;
>
> g_test_init(&argc, &argv, NULL);
> -qtest_add_func("/e1000/nop", nop);
>
> -qtest_start("-device e1000");
> -ret = g_test_run();
> +for (i = 0; i < ARRAY_SIZE(models); i++) {
> +char *path;
>
> -qtest_end();
> +path = g_strdup_printf("/%s/e1000/%s", qtest_get_arch(), models[i]);
> +g_test_add_data_func(path, models[i], test_device);
> +}
>
> -return ret;
> +return g_test_run();
> }
> --
> 1.9.3
>
>
Re: [Qemu-devel] [PATCH v3 3/3] e1000: remove broken support for 82573L
On Sun, Jun 1, 2014 at 1:33 PM, Gabriel L. Somlo wrote:
> Currently, e1000 support is based on the manual for the 8254xx
> model series. 82573x models are documented in a separate manual
> (see
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/pcie-gbe-controllers-open-source-manual.pdf)
> and the 82573L device ID no longer works correctly on either Linux
> (3.14.*) or Windows 7.
>
> This patch removes stale code claiming to support 82573L, cleaning
> up the code base for the remaining 8254xx model series.
>
> Signed-off-by: Gabriel Somlo
Reviewed-by: Peter Crosthwaite
> ---
> hw/net/e1000.c | 18 --
> tests/e1000-test.c | 1 -
> 2 files changed, 19 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 4e208e3..3ae5b43 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -70,8 +70,6 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
> /*
> * HW models:
> * E1000_DEV_ID_82540EM works with Windows, Linux, and OS X <= 10.8
> - * E1000_DEV_ID_82573L OK with windoze and Linux 2.6.22,
> - * appears to perform better than 82540EM, but breaks with Linux 2.6.18
> * E1000_DEV_ID_82544GC_COPPER appears to work; not well tested
> * E1000_DEV_ID_82545EM_COPPER works with Linux and OS X >= 10.6
> * Others never tested
> @@ -144,7 +142,6 @@ typedef struct E1000State_st {
> typedef struct E1000DeviceClass {
> PCIDeviceClass parent_class;
> uint16_t phy_id2;
> -bool is_8257xx;
> } E1000DeviceClass;
>
> #define TYPE_E1000_BASE "e1000-base"
> @@ -271,15 +268,9 @@ static void
> set_interrupt_cause(E1000State *s, int index, uint32_t val)
> {
> PCIDevice *d = PCI_DEVICE(s);
> -E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
> uint32_t pending_ints;
> uint32_t mit_delay;
>
> -if (val && edc->is_8257xx) {
> -/* hack only for 8257xx models */
> -val |= E1000_ICR_INT_ASSERTED;
> -}
> -
> s->mac_reg[ICR] = val;
>
> /*
> @@ -1581,7 +1572,6 @@ typedef struct E1000Info_st {
> uint16_t device_id;
> uint8_trevision;
> uint16_t phy_id2;
> -bool is_8257xx;
> } E1000Info;
>
> static void e1000_class_init(ObjectClass *klass, void *data)
> @@ -1598,7 +1588,6 @@ static void e1000_class_init(ObjectClass *klass, void
> *data)
> k->device_id = info->device_id;
> k->revision = info->revision;
> e->phy_id2 = info->phy_id2;
> -e->is_8257xx = info->is_8257xx;
> k->class_id = PCI_CLASS_NETWORK_ETHERNET;
> set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> dc->desc = "Intel Gigabit Ethernet";
> @@ -1634,13 +1623,6 @@ static const E1000Info e1000_devices[] = {
> .revision = 0x03,
> .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT,
> },
> -{
> -.name = "e1000-82573l",
> -.device_id = E1000_DEV_ID_82573L,
> -.revision = 0x03,
> -.phy_id2 = E1000_PHY_ID2_82573x,
> -.is_8257xx = true,
> -},
> };
>
> static const TypeInfo e1000_default_info = {
> diff --git a/tests/e1000-test.c b/tests/e1000-test.c
> index 53c41f8..81f164d 100644
> --- a/tests/e1000-test.c
> +++ b/tests/e1000-test.c
> @@ -33,7 +33,6 @@ static const char *models[] = {
> "e1000-82540em",
> "e1000-82544gc",
> "e1000-82545em",
> -"e1000-82573l",
> };
>
> int main(int argc, char **argv)
> --
> 1.9.3
>
>
Re: [Qemu-devel] [PATCH v3 0/3] e1000: allow model/device_id selection on command line
On Sun, Jun 1, 2014 at 1:33 PM, Gabriel L. Somlo wrote: > Allow selection of different card models from the qemu > command line, to better accomodate a wider range of guests. > > New in v3: > > - 1/3 and 2/3 from v2 now merged into a single patch (1/3), with: > - s/TYPE_E1000/TYPE_E1000_BASE/ as suggested by Stefan > - improved QOM-ification as suggested by Peter Crosthwaite > > - *OPTIONAL* patch to remove stale support for 8257xx (see commit blurb > in patch 3/3 for details > (this can be squashed on top of 1/3, but I'm including it separately > here for clarity, and as an RFC). > I think it's ok to merge as three separate patches organised the way you have done it. It means if someone comes along later and decides to repair the broken support they can revert just your last patch as starting point, rather than having to disentangle it from the QOMification work. The other alternative if you want to minimise churn, is to do your defeaturing first then QOMifiy but that doesn't do the repair guy any favours. Should probably still be three separate patches though. Regards, Peter > Thanks, > Gabriel > > > v2: > > - moved check for 8257x out of the way of QOM, as suggested by > Michael (patch 1/3) > > - resolved "Signed-off-by" misunderstanding and miscellaneous style > issues (patch 2/3) > > - modified e1000 test to check for all supported models, as suggested > by Andreas (patch 3/3). I used eepro100-test.c as an example for > this change. > > > Gabriel L. Somlo (3): > e1000: allow command-line selection of card model > tests: e1000: test additional device IDs > e1000: remove broken support for 82573L > > hw/net/e1000.c | 110 > +++- > hw/net/e1000_regs.h | 6 +++ > tests/e1000-test.c | 33 > 3 files changed, 114 insertions(+), 35 deletions(-) > > -- > 1.9.3 > >
[Qemu-devel] Nested KVM is weird?
Hi, I managed to run VMWare ESXi hypervisor (type 1) inside KVM. From there, I can install & run nested guest Linux on this ESXi. However, I find this very strange: I put some printk() at the top of function nested_vmx_run(), which should run to handle nested KVM. Below is the definition of nested_vmx_run() in vmx.c /* * nested_vmx_run() handles a nested entry, i.e., a VMLAUNCH or VMRESUME on L1 * for running an L2 nested guest. */ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) What I cannot understand is that this function is never hit, because my printk() never prints out anything to dmesg output. So this means ESXi never uses VMResume/VMLaunch? How is this possible, because it uses VMX for its implementation? I am banging my head into keyboard to figure out why. Any idea, please? Thanks, Jun
Re: [Qemu-devel] Nested KVM is weird?
On Sun, Jun 01, 2014 at 05:54:25PM +0700, Jun Koi wrote: > So this means ESXi never uses VMResume/VMLaunch? How is this > possible, because it uses VMX for its implementation? ESXi will fall back to binary translation if it decides that it cannot use VMX for some reason. Look at the L2's log file inside ESXi and you will likely find some errors related to running nested, MSRs KVM does not emulate properly, or something else that causes ESXi to use binary translation for its L2. Cheers, Muli
[Qemu-devel] [PATCH] readline: Clear screen on form feed.
Signed-off-by: Hani Benhabiles
---
util/readline.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/util/readline.c b/util/readline.c
index 8baec55..08d07e3 100644
--- a/util/readline.c
+++ b/util/readline.c
@@ -345,6 +345,12 @@ static void readline_completion(ReadLineState *rs)
}
}
+static void readline_clear_screen(ReadLineState *rs)
+{
+rs->printf_func(rs->opaque, "\033[2J\033[1;1H");
+readline_show_prompt(rs);
+}
+
/* return true if command handled */
void readline_handle_byte(ReadLineState *rs, int ch)
{
@@ -363,6 +369,9 @@ void readline_handle_byte(ReadLineState *rs, int ch)
case 9:
readline_completion(rs);
break;
+case 12:
+readline_clear_screen(rs);
+break;
case 10:
case 13:
rs->cmd_buf[rs->cmd_buf_size] = '\0';
--
1.8.3.2
Re: [Qemu-devel] [PATCH] trace: Replace fprintf with error_report and print location
On 05/31/2014 12:08 AM, Lluís Vilanova wrote:
> Alexey Kardashevskiy writes:
>
>> On 05/29/2014 10:42 PM, Lluís Vilanova wrote:
>>> Alexey Kardashevskiy writes:
>>>
This replaces fprintf(stderr) with error_report.
This prints line number of the trace which does not exist or is not
traceable.
>>>
>>> A little nit pick; it shows an error when some of the events in the list of
>>> events to enable (not the trace) does not exist or is not traceable.
>
>
>> Sorry, I am not following you. It shows me this:
>
>> qemu-system-ppc64:/home/aik/qemu_trace_events:75: WARNING: trace event
>> 'mememe' does not exist
>
>> and continues. This exactly what I wanted.
>
> Yes, that's right. All I'm saying is that the commit message is a little bit
> misleading. These are not the line numbers of a trace, but the line numbers
> of a
> list of events to enable when QEMU starts.
Like this? Thanks.
===
This replaces fprintf(stderr) with error_report.
This prints the line number of a list of events which does not exist or is
not traceable.
This moves local variables to the beginning of the function because of
the QEMU coding style.
===
>
>
> Thanks,
> Lluis
>
>
>>>
>>>
>>> Thanks,
>>> Lluis
>>>
>>>
This moves local variables to the beginning of the function because of
the QEMU coding style.
>>>
Suggested-by: Lluís Vilanova
Signed-off-by: Alexey Kardashevskiy
---
>>>
Lluís, or it is s/Suggested-by/From/ ?
>>>
Stefan, this is made on top of "trace: Replace error with warning if event
is not defined"
>>>
---
trace/control.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
>>>
diff --git a/trace/control.c b/trace/control.c
index 4aa02cf..4ee2bd2 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -8,7 +8,7 @@
*/
>>>
#include "trace/control.h"
-
+#include "qemu/error-report.h"
>>>
TraceEvent *trace_event_name(const char *name)
{
@@ -81,18 +81,24 @@ TraceEvent *trace_event_pattern(const char *pat,
TraceEvent *ev)
>>>
void trace_backend_init_events(const char *fname)
{
+Location loc;
+FILE *fp;
+char line_buf[1024];
+size_t line_idx = 0;
+
if (fname == NULL) {
return;
}
>>>
-FILE *fp = fopen(fname, "r");
+loc_push_none(&loc);
+loc_set_file(fname, 0);
+fp = fopen(fname, "r");
if (!fp) {
-fprintf(stderr, "error: could not open trace events file '%s':
%s\n",
-fname, strerror(errno));
+error_report("%s", strerror(errno));
exit(1);
}
-char line_buf[1024];
while (fgets(line_buf, sizeof(line_buf), fp)) {
+loc_set_file(fname, ++line_idx);
size_t len = strlen(line_buf);
if (len > 1) { /* skip empty lines */
line_buf[len - 1] = '\0';
@@ -111,13 +117,11 @@ void trace_backend_init_events(const char *fname)
} else {
TraceEvent *ev = trace_event_name(line_ptr);
if (ev == NULL) {
-fprintf(stderr,
-"WARNING: trace event '%s' does not exist\n",
-line_ptr);
+error_report("WARNING: trace event '%s' does not
exist",
+ line_ptr);
} else if (!trace_event_get_state_static(ev)) {
-fprintf(stderr,
-"WARNING: trace event '%s' is not
traceable\n",
-line_ptr);
+error_report("WARNING: trace event '%s' is not
traceable\n",
+ line_ptr);
} else {
trace_event_set_state_dynamic(ev, enable);
}
@@ -125,8 +129,9 @@ void trace_backend_init_events(const char *fname)
}
}
if (fclose(fp) != 0) {
-fprintf(stderr, "error: closing file '%s': %s\n",
-fname, strerror(errno));
+loc_set_file(fname, 0);
+error_report("%s", strerror(errno));
exit(1);
}
+loc_pop(&loc);
}
--
Alexey
[Qemu-devel] [RFC PATCH] kvm: Enable -cpu option to hide KVM
The latest Nvidia driver (337.88) specifically checks for KVM as the
hypervisor and reports Code 43 for the driver in a Windows guest when
found. Removing or changing the KVM signature is sufficient to allow
the driver to load. This patch adds an option to easily allow the KVM
hypervisor signature to be hidden using '-cpu no-kvm'.
Signed-off-by: Alex Williamson
---
target-i386/cpu-qom.h |1 +
target-i386/cpu.c |1 +
target-i386/kvm.c | 28 +++-
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index e9b3d57..99bb059 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -87,6 +87,7 @@ typedef struct X86CPU {
bool hyperv_time;
bool check_cpuid;
bool enforce_cpuid;
+bool no_kvm;
/* if true the CPUID code directly forward host cache leaves to the guest
*/
bool cache_info_passthrough;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 042a48d..8e6ce9c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2792,6 +2792,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
+DEFINE_PROP_BOOL("no-kvm", X86CPU, no_kvm, false),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d894ef..920898e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -528,23 +528,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
has_msr_hv_hypercall = true;
}
-memcpy(signature, "KVMKVMKVM\0\0\0", 12);
-c = &cpuid_data.entries[cpuid_i++];
-c->function = KVM_CPUID_SIGNATURE | kvm_base;
-c->eax = 0;
-c->ebx = signature[0];
-c->ecx = signature[1];
-c->edx = signature[2];
+if (!cpu->no_kvm) {
+memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+c = &cpuid_data.entries[cpuid_i++];
+c->function = KVM_CPUID_SIGNATURE | kvm_base;
+c->eax = 0;
+c->ebx = signature[0];
+c->ecx = signature[1];
+c->edx = signature[2];
-c = &cpuid_data.entries[cpuid_i++];
-c->function = KVM_CPUID_FEATURES | kvm_base;
-c->eax = env->features[FEAT_KVM];
+c = &cpuid_data.entries[cpuid_i++];
+c->function = KVM_CPUID_FEATURES | kvm_base;
+c->eax = env->features[FEAT_KVM];
-has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
+has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
-has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
+has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
-has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
+has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
+}
cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
Re: [Qemu-devel] Nested KVM is weird?
On Sun, Jun 1, 2014 at 5:59 PM, Muli Ben-Yehuda wrote: > On Sun, Jun 01, 2014 at 05:54:25PM +0700, Jun Koi wrote: > > > So this means ESXi never uses VMResume/VMLaunch? How is this > > possible, because it uses VMX for its implementation? > > ESXi will fall back to binary translation if it decides that it cannot > use VMX for some reason. Look at the L2's log file inside ESXi and you > will likely find some errors related to running nested, MSRs KVM does > not emulate properly, or something else that causes ESXi to use binary > translation for its L2. > now i noticed in dmesg output, there is one message with content: " VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL does not work properly. Using workaround" (1) do you think this VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL is the reason why ESXi falls back to binary translation? (3) how hard it is to fix this issue, so we have complete support for ESXi? thanks, Jun
Re: [Qemu-devel] [PATCH] thread-pool: fix deadlock when callbacks depends on each other
Il 31/05/2014 20:29, Marcin Gibuła ha scritto:
@@ -185,6 +191,14 @@ restart:
}
if (elem->state == THREAD_DONE && elem->common.cb) {
QLIST_REMOVE(elem, all);
+/* If more completed requests are waiting, notifier needs
+ * to be rearmed so callback can progress with aio_pool().
+ */
+pool->pending_completions--;
+if (pool->pending_completions) {
+event_notifier_set(notifier);
+}
+
/* Read state before ret. */
smp_rmb();
elem->common.cb(elem->common.opaque, elem->ret);
Good catch! The main problem with the patch is that you need to use
atomic_inc/atomic_dec to increment and decrement pool->pending_completions.
Secondarily, event_notifier_set is pretty heavy-weight, does it work if
you wrap the loop like this?
restart:
QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
...
}
if (pool->pending_completions) {
goto restart;
}
event_notifier_test_and_clear(notifier);
if (pool->pending_completions) {
event_notifier_set(notifier);
goto restart;
}
Finally, the same bug is also in block/linux-aio.c and block/win32-aio.c.
Paolo
Re: [Qemu-devel] When should management tools use q35?
Il 01/06/2014 03:22, Cole Robinson ha scritto: Hi all, There was a bit of discussion recently about exposing q35 in virt-manager: http://www.redhat.com/archives/virt-tools-list/2014-May/msg1.html In that thread, Gerd suggested a 'rule of thumb' to use q35 for any guest OS released in 2010 or later. Sounds reasonable to me, but I'm curious if anyone else wants to weigh in. Is q35 in 'good enough' shape to turn on a default like that? Does it still block migration? (pretty sure that was fixed, but I haven't confirmed) AHCI migration still does not work reliably. I think that we could change it to block migration only if you have IDE drives, but still means that exposing q35 in virt-manager might be problematic. Paolo
Re: [Qemu-devel] [RFC PATCH] kvm: Enable -cpu option to hide KVM
Il 01/06/2014 18:25, Alex Williamson ha scritto:
The latest Nvidia driver (337.88) specifically checks for KVM as the
hypervisor and reports Code 43 for the driver in a Windows guest when
found. Removing or changing the KVM signature is sufficient to allow
the driver to load. This patch adds an option to easily allow the KVM
hypervisor signature to be hidden using '-cpu no-kvm'.
Signed-off-by: Alex Williamson
It's really a nit, but I think "kvm=no" is preferrable (more consistent
with how hyper-v leaves are enabled).
Paolo
---
target-i386/cpu-qom.h |1 +
target-i386/cpu.c |1 +
target-i386/kvm.c | 28 +++-
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index e9b3d57..99bb059 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -87,6 +87,7 @@ typedef struct X86CPU {
bool hyperv_time;
bool check_cpuid;
bool enforce_cpuid;
+bool no_kvm;
/* if true the CPUID code directly forward host cache leaves to the guest
*/
bool cache_info_passthrough;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 042a48d..8e6ce9c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2792,6 +2792,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
+DEFINE_PROP_BOOL("no-kvm", X86CPU, no_kvm, false),
DEFINE_PROP_END_OF_LIST()
};
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d894ef..920898e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -528,23 +528,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
has_msr_hv_hypercall = true;
}
-memcpy(signature, "KVMKVMKVM\0\0\0", 12);
-c = &cpuid_data.entries[cpuid_i++];
-c->function = KVM_CPUID_SIGNATURE | kvm_base;
-c->eax = 0;
-c->ebx = signature[0];
-c->ecx = signature[1];
-c->edx = signature[2];
+if (!cpu->no_kvm) {
+memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+c = &cpuid_data.entries[cpuid_i++];
+c->function = KVM_CPUID_SIGNATURE | kvm_base;
+c->eax = 0;
+c->ebx = signature[0];
+c->ecx = signature[1];
+c->edx = signature[2];
-c = &cpuid_data.entries[cpuid_i++];
-c->function = KVM_CPUID_FEATURES | kvm_base;
-c->eax = env->features[FEAT_KVM];
+c = &cpuid_data.entries[cpuid_i++];
+c->function = KVM_CPUID_FEATURES | kvm_base;
+c->eax = env->features[FEAT_KVM];
-has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
+has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
-has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
+has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
-has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
+has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
+}
cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] thread-pool: fix deadlock when callbacks depends on each other
Good catch! The main problem with the patch is that you need to use
atomic_inc/atomic_dec to increment and decrement pool->pending_completions.
Ok.
Secondarily, event_notifier_set is pretty heavy-weight, does it work if
you wrap the loop like this?
restart:
QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
...
}
if (pool->pending_completions) {
goto restart;
}
event_notifier_test_and_clear(notifier);
if (pool->pending_completions) {
event_notifier_set(notifier);
goto restart;
}
I'll test it tomorrow. I assume you want to avoid calling
event_notifier_set() until function is reentered via aio_pool?
> Finally, the same bug is also in block/linux-aio.c and
> block/win32-aio.c.
I can try with linux-aio, but my knowledge of windows api is zero...
--
mg
Re: [Qemu-devel] [PATCH] trace: Replace fprintf with error_report and print location
Alexey Kardashevskiy writes:
> On 05/31/2014 12:08 AM, Lluís Vilanova wrote:
>> Alexey Kardashevskiy writes:
>>
>>> On 05/29/2014 10:42 PM, Lluís Vilanova wrote:
Alexey Kardashevskiy writes:
> This replaces fprintf(stderr) with error_report.
> This prints line number of the trace which does not exist or is not
> traceable.
A little nit pick; it shows an error when some of the events in the list of
events to enable (not the trace) does not exist or is not traceable.
>>
>>
>>> Sorry, I am not following you. It shows me this:
>>
>>> qemu-system-ppc64:/home/aik/qemu_trace_events:75: WARNING: trace event
>>> 'mememe' does not exist
>>
>>> and continues. This exactly what I wanted.
>>
>> Yes, that's right. All I'm saying is that the commit message is a little bit
>> misleading. These are not the line numbers of a trace, but the line numbers
>> of a
>> list of events to enable when QEMU starts.
> Like this? Thanks.
> ===
> This replaces fprintf(stderr) with error_report.
> This prints the line number of a list of events which does not exist or is
> not traceable.
> This moves local variables to the beginning of the function because of
> the QEMU coding style.
> ===
I think there is no need to describe what that function is doing. Instead I
would say something like:
Replaces fprintf(stdeer) with error_report.
Also moves some local variables to the beginning of the function to comply
with QEMU's coding style.
Thanks,
Lluis
>>
>>
>> Thanks,
>> Lluis
>>
>>
Thanks,
Lluis
> This moves local variables to the beginning of the function because of
> the QEMU coding style.
> Suggested-by: Lluís Vilanova
> Signed-off-by: Alexey Kardashevskiy
> ---
> Lluís, or it is s/Suggested-by/From/ ?
> Stefan, this is made on top of "trace: Replace error with warning if
> event is not defined"
> ---
> trace/control.c | 31 ++-
> 1 file changed, 18 insertions(+), 13 deletions(-)
> diff --git a/trace/control.c b/trace/control.c
> index 4aa02cf..4ee2bd2 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -8,7 +8,7 @@
> */
> #include "trace/control.h"
> -
> +#include "qemu/error-report.h"
> TraceEvent *trace_event_name(const char *name)
> {
> @@ -81,18 +81,24 @@ TraceEvent *trace_event_pattern(const char *pat,
> TraceEvent *ev)
> void trace_backend_init_events(const char *fname)
> {
> +Location loc;
> +FILE *fp;
> +char line_buf[1024];
> +size_t line_idx = 0;
> +
> if (fname == NULL) {
> return;
> }
> -FILE *fp = fopen(fname, "r");
> +loc_push_none(&loc);
> +loc_set_file(fname, 0);
> +fp = fopen(fname, "r");
> if (!fp) {
> -fprintf(stderr, "error: could not open trace events file '%s':
> %s\n",
> -fname, strerror(errno));
> +error_report("%s", strerror(errno));
> exit(1);
> }
> -char line_buf[1024];
> while (fgets(line_buf, sizeof(line_buf), fp)) {
> +loc_set_file(fname, ++line_idx);
> size_t len = strlen(line_buf);
> if (len > 1) { /* skip empty lines */
> line_buf[len - 1] = '\0';
> @@ -111,13 +117,11 @@ void trace_backend_init_events(const char *fname)
> } else {
> TraceEvent *ev = trace_event_name(line_ptr);
> if (ev == NULL) {
> -fprintf(stderr,
> -"WARNING: trace event '%s' does not exist\n",
> -line_ptr);
> +error_report("WARNING: trace event '%s' does not
> exist",
> + line_ptr);
> } else if (!trace_event_get_state_static(ev)) {
> -fprintf(stderr,
> -"WARNING: trace event '%s' is not
> traceable\n",
> -line_ptr);
> +error_report("WARNING: trace event '%s' is not
> traceable\n",
> + line_ptr);
> } else {
> trace_event_set_state_dynamic(ev, enable);
> }
> @@ -125,8 +129,9 @@ void trace_backend_init_events(const char *fname)
> }
> }
> if (fclose(fp) != 0) {
> -fprintf(stderr, "error: closing file '%s': %s\n",
> -fname, strerror(errno));
> +loc_set_file(fname, 0);
> +error_report("%s", strerror(errno));
> exit(1);
> }
> +loc_pop(&loc);
> }
> --
> Alexey
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
Re: [Qemu-devel] [RFC PATCH] kvm: Enable -cpu option to hide KVM
On Sun, 2014-06-01 at 20:29 +0200, Paolo Bonzini wrote:
> Il 01/06/2014 18:25, Alex Williamson ha scritto:
> > The latest Nvidia driver (337.88) specifically checks for KVM as the
> > hypervisor and reports Code 43 for the driver in a Windows guest when
> > found. Removing or changing the KVM signature is sufficient to allow
> > the driver to load. This patch adds an option to easily allow the KVM
> > hypervisor signature to be hidden using '-cpu no-kvm'.
> >
> > Signed-off-by: Alex Williamson
>
> It's really a nit, but I think "kvm=no" is preferrable (more consistent
> with how hyper-v leaves are enabled).
Happy to oblige, but I'm not sure what I'm looking for. We enably
hyper-v leaves if hyperv_enabled(), which seems to boil down to the kvm
kernel supporting KVM_CAP_HYPERV and one or more cpu->hyperv_foo
features enabled. What's the commandline option I'm looking for that
has some sort of hyper-v=on|off? Thanks,
Alex
> > ---
> > target-i386/cpu-qom.h |1 +
> > target-i386/cpu.c |1 +
> > target-i386/kvm.c | 28 +++-
> > 3 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> > index e9b3d57..99bb059 100644
> > --- a/target-i386/cpu-qom.h
> > +++ b/target-i386/cpu-qom.h
> > @@ -87,6 +87,7 @@ typedef struct X86CPU {
> > bool hyperv_time;
> > bool check_cpuid;
> > bool enforce_cpuid;
> > +bool no_kvm;
> >
> > /* if true the CPUID code directly forward host cache leaves to the
> > guest */
> > bool cache_info_passthrough;
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 042a48d..8e6ce9c 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2792,6 +2792,7 @@ static Property x86_cpu_properties[] = {
> > DEFINE_PROP_BOOL("hv-time", X86CPU, hyperv_time, false),
> > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
> > DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> > +DEFINE_PROP_BOOL("no-kvm", X86CPU, no_kvm, false),
> > DEFINE_PROP_END_OF_LIST()
> > };
> >
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 0d894ef..920898e 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -528,23 +528,25 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > has_msr_hv_hypercall = true;
> > }
> >
> > -memcpy(signature, "KVMKVMKVM\0\0\0", 12);
> > -c = &cpuid_data.entries[cpuid_i++];
> > -c->function = KVM_CPUID_SIGNATURE | kvm_base;
> > -c->eax = 0;
> > -c->ebx = signature[0];
> > -c->ecx = signature[1];
> > -c->edx = signature[2];
> > +if (!cpu->no_kvm) {
> > +memcpy(signature, "KVMKVMKVM\0\0\0", 12);
> > +c = &cpuid_data.entries[cpuid_i++];
> > +c->function = KVM_CPUID_SIGNATURE | kvm_base;
> > +c->eax = 0;
> > +c->ebx = signature[0];
> > +c->ecx = signature[1];
> > +c->edx = signature[2];
> >
> > -c = &cpuid_data.entries[cpuid_i++];
> > -c->function = KVM_CPUID_FEATURES | kvm_base;
> > -c->eax = env->features[FEAT_KVM];
> > +c = &cpuid_data.entries[cpuid_i++];
> > +c->function = KVM_CPUID_FEATURES | kvm_base;
> > +c->eax = env->features[FEAT_KVM];
> >
> > -has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
> > +has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
> >
> > -has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
> > +has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
> >
> > -has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
> > +has_msr_kvm_steal_time = c->eax & (1 << KVM_FEATURE_STEAL_TIME);
> > +}
> >
> > cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
Re: [Qemu-devel] [PATCH qom v1 1/1] qom: object: remove parent pointer when unparenting
Ping!
On Tue, May 27, 2014 at 10:39 AM, Peter Crosthwaite
wrote:
> Certain parts of the QOM framework test this pointer to determine if
> an object is parented. Nuke it when the object is unparented to allow
> for reuse of an object after unparenting.
>
> Signed-off-by: Peter Crosthwaite
> ---
>
> qom/object.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/qom/object.c b/qom/object.c
> index e42b254..8319e89 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -402,6 +402,7 @@ void object_unparent(Object *obj)
> if (obj->parent) {
> object_property_del_child(obj->parent, obj, NULL);
> }
> +obj->parent = NULL;
> object_unref(obj);
> }
>
> --
> 1.9.3.1.ga73a6ad
>
Re: [Qemu-devel] [PATCH v1 14/16] target-arm: A64: Emulate the SMC insn
On Fri, May 30, 2014 at 05:28:29PM +1000, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias"
>
> Signed-off-by: Edgar E. Iglesias
> ---
> target-arm/cpu.h | 1 +
> target-arm/helper-a64.c| 1 +
> target-arm/helper.c| 6 ++
> target-arm/helper.h| 1 +
> target-arm/internals.h | 6 ++
> target-arm/op_helper.c | 11 +++
> target-arm/translate-a64.c | 10 ++
> 7 files changed, 36 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1a26ed4..b3631f2 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -52,6 +52,7 @@
> #define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */
> #define EXCP_STREX 10
> #define EXCP_HVC11 /* HyperVisor Call */
> +#define EXCP_SMC12 /* Secure Monitor Call */
>
> #define ARMV7M_EXCP_RESET 1
> #define ARMV7M_EXCP_NMI 2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 974fa66..3894a6f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
> case EXCP_UDEF:
> case EXCP_SWI:
> case EXCP_HVC:
> +case EXCP_SMC:
> env->cp15.esr_el[new_el] = env->exception.syndrome;
> break;
> case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 5b2070c..35091ea 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3298,6 +3298,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned
> int excp_idx)
> case EXCP_HVC:
> target_el = MAX(target_el, 2);
> break;
> +case EXCP_SMC:
> +target_el = 3;
> +if (env->cp15.hcr_el2 & HCR_TSC) {
The HCR.TSC routing of SMC to EL2 only applies in NS EL1 mode,
will fix for v2 as:
case EXCP_SMC:
target_el = 3;
if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
target_el = 2;
}
break;
> +target_el = 2;
> +}
> +break;
> }
> return target_el;
> }
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index fb711be..6c3d84d 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
> DEF_HELPER_1(wfi, void, env)
> DEF_HELPER_1(wfe, void, env)
> DEF_HELPER_2(hvc, void, env, i32)
> +DEF_HELPER_2(smc, void, env, i32)
>
> DEF_HELPER_3(cpsr_write, void, env, i32, i32)
> DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index b08381c..e50a68e 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -54,6 +54,7 @@ static const char * const excnames[] = {
> [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
> [EXCP_STREX] = "QEMU intercept of STREX",
> [EXCP_HVC] = "Hypervisor Call",
> +[EXCP_SMC] = "Secure Monitor Call",
> };
>
> static inline void arm_log_exception(int idx)
> @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
> return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
> }
>
> +static inline uint32_t syn_aa64_smc(uint32_t imm16)
> +{
> +return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
> +}
> +
> static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
> {
> return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0x)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 6bf34b0..6840828 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -405,6 +405,17 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
> raise_exception(env, EXCP_HVC);
> }
>
> +void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
> +{
> +/* We've already checked that EL3 exists at translation time. */
> +if (env->cp15.scr_el3 & SCR_SMD) {
> +env->exception.syndrome = syn_uncategorized();
> +raise_exception(env, EXCP_UDEF);
> +}
> +env->exception.syndrome = syndrome;
> +raise_exception(env, EXCP_SMC);
> +}
> +
> void HELPER(exception_return)(CPUARMState *env)
> {
> int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 3981ee1..a02fe06 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1451,6 +1451,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
> gen_helper_hvc(cpu_env, tmp);
> tcg_temp_free_i32(tmp);
> break;
> +case 3:
> +if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
> +unallocated_encoding(s);
> +break;
> +}
> +tmp = tcg_const_i32(syn_aa64_smc(imm16));
> +gen_a64_set_pc_im(s->pc);
> +gen_helper_smc(cpu_env, tmp);
> +tcg_temp_free_i32(tmp);
> +break;
>
Re: [Qemu-devel] [PATCH memory v2 6/9] memory: MemoryRegion: QOMify
On Wed, May 28, 2014 at 2:24 AM, Paolo Bonzini wrote:
> Il 27/05/2014 11:02, Peter Crosthwaite ha scritto:
>
>>
>> +void memory_region_destroy(MemoryRegion *mr)
>> +{
>> +/*FIXME: whatever the opposite of object initialize is, do it here */
>> +memory_region_finalize(OBJECT(mr));
>> +}
>> +
>
>
> That's object_unref.
>
Fixed. Thanks.
Regards,
Peter
> Paolo
>
Re: [Qemu-devel] [PATCH memory v2 7/9] memory: MemoryRegion: Add container and addr props
On Tue, May 27, 2014 at 7:03 PM, Peter Crosthwaite
wrote:
> Expose the already existing .parent and .addr fields as QOM properties.
> .parent (i.e. the field describing the memory region that contains this
> one in Memory hierachy) is renamed "container". This is to avoid
> confusion with the owner field, which is much more akin to an actual QOM
> parent.
>
> Setting the .parent ("container") will cause the memory subregion adding
> to happen. Nullifying or changing the .parent will delete or relocate
> (to a different container) the subregion resp.
>
> Setting or changing the address will relocate the memory region.
>
> Signed-off-by: Peter Crosthwaite
> ---
> changed since v1:
> Converted container to low level link property and added subregion setup.
>
> memory.c | 108
> +++
> 1 file changed, 108 insertions(+)
>
> diff --git a/memory.c b/memory.c
> index d9d3c07..a95bb1e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -16,6 +16,7 @@
> #include "exec/memory.h"
> #include "exec/address-spaces.h"
> #include "exec/ioport.h"
> +#include "qapi/visitor.h"
> #include "qemu/bitops.h"
> #include "qom/object.h"
> #include "trace.h"
> @@ -861,9 +862,104 @@ void memory_region_init(MemoryRegion *mr,
> mr->name = g_strdup(name);
> }
>
> +static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +Error *local_err = NULL;
> +uint64_t value = mr->addr;
> +
> +visit_type_uint64(v, &value, name, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +}
> +}
> +
> +static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +Error *local_err = NULL;
> +uint64_t value;
> +
> +visit_type_uint64(v, &value, name, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> +memory_region_set_address(mr, value);
> +}
> +
> +static void memory_region_set_container(Object *obj, Visitor *v, void
> *opaque,
> +const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +Error *local_err = NULL;
> +MemoryRegion *old_parent = mr->parent;
> +MemoryRegion *new_parent = NULL;
> +char *path = NULL;
> +
> +visit_type_str(v, &path, name, &local_err);
> +
> +if (!local_err && strcmp(path, "") != 0) {
> +new_parent = MEMORY_REGION(object_resolve_link(obj, name, path,
> + &local_err));
> +}
> +
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> +object_ref(OBJECT(new_parent));
> +
> +memory_region_transaction_begin();
> +memory_region_ref(mr);
> +if (old_parent) {
> +memory_region_del_subregion(old_parent, mr);
> +}
> +mr->parent = new_parent;
> +if (new_parent) {
> +do_memory_region_add_subregion_common(mr);
> +}
> +memory_region_unref(mr);
> +memory_region_transaction_commit();
> +
> +object_unref(OBJECT(old_parent));
> +}
> +
> +static void memory_region_get_container(Object *obj, Visitor *v, void
> *opaque,
> +const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +gchar *path = (gchar *)"";
> +
> +if (mr->parent) {
> +path = object_get_canonical_path(OBJECT(mr->parent));
> +}
> +visit_type_str(v, &path, name, errp);
> +if (mr->parent) {
> +g_free(path);
> +}
> +}
> +
> +static void memory_region_release_container(Object *obj, const char *name,
> +void *opaque)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +
> +if (mr->parent) {
> +memory_region_del_subregion(mr->parent, mr);
> +object_unref(OBJECT(mr->parent));
> +}
> +}
> +
> static void memory_region_initfn(Object *obj)
> {
> MemoryRegion *mr = MEMORY_REGION(obj);
> +gchar *container_link_type = g_strdup_printf("link<%s>",
> + TYPE_MEMORY_REGION);
Since TYPE_MEMORY_REGION is a literal string constant, this can be
done with regular "" "" style string concatenation. Dropped the strdup
in V3.
Regards,
Peter
>
> mr->ops = &unassigned_mem_ops;
> mr->enabled = true;
> @@ -872,6 +968,18 @@ static void memory_region_initfn(Object *obj)
> QTAILQ_INIT(&mr->subregions);
> memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
> QTAILQ_INIT(&mr->coalesced);
> +
> +object_property_add(OBJECT(mr), "container", container_link_type,
> +memory_region_get_container,
> +memory_r
Re: [Qemu-devel] [PATCH memory v2 8/9] memory: MemoryRegion: Add may-overlap and priority props
On Tue, May 27, 2014 at 7:04 PM, Peter Crosthwaite
wrote:
> QOM propertyify the .may-overlap and .priority fields. The setters
> will re-add the memory as a subregion if needed (i.e. the values change
> when the memory region is already contained).
>
> Signed-off-by: Peter Crosthwaite
> ---
> changed since v1:
> Converted priority to signed type
>
> include/exec/memory.h | 2 +-
> memory.c | 57
> +++
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 371c066..117c0d3 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -157,7 +157,7 @@ struct MemoryRegion {
> bool flush_coalesced_mmio;
> MemoryRegion *alias;
> hwaddr alias_offset;
> -int priority;
> +int32_t priority;
> bool may_overlap;
> QTAILQ_HEAD(subregions, MemoryRegion) subregions;
> QTAILQ_ENTRY(MemoryRegion) subregions_link;
> diff --git a/memory.c b/memory.c
> index a95bb1e..ee761a2 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -955,6 +955,55 @@ static void memory_region_release_container(Object *obj,
> const char *name,
> }
> }
>
> +static void memory_region_get_priority(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +Error *local_err = NULL;
> +uint32_t value = mr->addr;
That's a copy paste error. Fixed. Also change type to int32_t to
support signed priorities via QOM setters/getters.
Regards,
Peter
> +
> +visit_type_uint32(v, &value, name, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +}
> +}
> +
> +static void memory_region_set_priority(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +Error *local_err = NULL;
> +uint32_t value;
> +
> +visit_type_uint32(v, &value, name, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> +if (mr->priority != value) {
> +mr->priority = value;
> +memory_region_readd_subregion(mr);
> +}
> +}
> +
> +static bool memory_region_get_may_overlap(Object *obj, Error **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +
> +return mr->may_overlap;
> +}
> +
> +static void memory_region_set_may_overlap(Object *obj, bool value, Error
> **errp)
> +{
> +MemoryRegion *mr = MEMORY_REGION(obj);
> +
> +if (mr->may_overlap != value) {
> +mr->may_overlap = value;
> +memory_region_readd_subregion(mr);
> +}
> +}
> +
> static void memory_region_initfn(Object *obj)
> {
> MemoryRegion *mr = MEMORY_REGION(obj);
> @@ -980,6 +1029,14 @@ static void memory_region_initfn(Object *obj)
> memory_region_get_addr,
> memory_region_set_addr,
> NULL, NULL, &error_abort);
> +object_property_add(OBJECT(mr), "priority", "uint32",
> +memory_region_get_priority,
> +memory_region_set_priority,
> +NULL, NULL, &error_abort);
> +object_property_add_bool(OBJECT(mr), "may-overlap",
> + memory_region_get_may_overlap,
> + memory_region_set_may_overlap,
> + &error_abort);
> }
>
> static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> --
> 1.9.3.1.ga73a6ad
>
[Qemu-devel] [PATCH memory v3 0/9] Memory Region QOMification
Hi Paolo, Andreas, This patch series QOMifies Memory regions. This is the Memory API specific subset of patches forming part of the Memory/GPIO/Sysbus QOMification. I think Paolo already has P1 enqeued. Including for ease of review. some QOM patches in P2-3 that cut down on later boilerplate. TBH I can live without them, if they not liked but they make life better IMO. For fuller context please see: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03265.html Changed since v2: Use object unref to finalize MR (Paolo review) Fixed priority prop getter Changed prioirty to signed type in QOM getters/setterts Changed since v1: Split into subset series. Converted container link into low level link. Misc finer tweaks and patch re-orderings. Peter Crosthwaite (9): memory: Simplify mr_add_subregion() if-else qom: object: Ignore refs/unrefs of NULL qom: Publish object_resolve_link memory: Coreify subregion add functionality memory: MemoryRegion: factor out memory region re-adder memory: MemoryRegion: QOMify memory: MemoryRegion: Add container and addr props memory: MemoryRegion: Add may-overlap and priority props memory: MemoryRegion: Add size property include/exec/memory.h | 8 +- include/qom/object.h | 16 +++ memory.c | 307 +++--- qom/object.c | 27 ++--- 4 files changed, 298 insertions(+), 60 deletions(-) -- 2.0.0
[Qemu-devel] [PATCH memory v3 1/9] memory: Simplify mr_add_subregion() if-else
This if else is not needed. The previous call to memory_region_add
(whether _overlap or not) will always set priority and may_overlap
to desired values. And its not possible to get here without having
called memory_region_add_subregion due to the null guard on parent.
So we can just directly call memory_region_add_subregion_common.
Signed-off-by: Peter Crosthwaite
---
memory.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/memory.c b/memory.c
index 3f1df23..1352881 100644
--- a/memory.c
+++ b/memory.c
@@ -1501,8 +1501,6 @@ void memory_region_set_enabled(MemoryRegion *mr, bool
enabled)
void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
{
MemoryRegion *parent = mr->parent;
-int priority = mr->priority;
-bool may_overlap = mr->may_overlap;
if (addr == mr->addr || !parent) {
mr->addr = addr;
@@ -1512,11 +1510,7 @@ void memory_region_set_address(MemoryRegion *mr, hwaddr
addr)
memory_region_transaction_begin();
memory_region_ref(mr);
memory_region_del_subregion(parent, mr);
-if (may_overlap) {
-memory_region_add_subregion_overlap(parent, addr, mr, priority);
-} else {
-memory_region_add_subregion(parent, addr, mr);
-}
+memory_region_add_subregion_common(parent, addr, mr);
memory_region_unref(mr);
memory_region_transaction_commit();
}
--
2.0.0
[Qemu-devel] [PATCH memory v3 2/9] qom: object: Ignore refs/unrefs of NULL
Just do nothing if passed NULL for a ref or unref. This avoids
call sites that manage a combination of NULL or non-NULL pointers
having to add iffery around every ref and unref.
Signed-off-by: Peter Crosthwaite
---
qom/object.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index e42b254..ec5adf4 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -714,11 +714,17 @@ GSList *object_class_get_list(const char *implements_type,
void object_ref(Object *obj)
{
+if (!obj) {
+return;
+}
atomic_inc(&obj->ref);
}
void object_unref(Object *obj)
{
+if (!obj) {
+return;
+}
g_assert(obj->ref > 0);
/* parent always holds a reference to its children */
@@ -1119,13 +1125,9 @@ static void object_set_link_property(Object *obj,
Visitor *v, void *opaque,
return;
}
-if (new_target) {
-object_ref(new_target);
-}
+object_ref(new_target);
*child = new_target;
-if (old_target != NULL) {
-object_unref(old_target);
-}
+object_unref(old_target);
}
static void object_release_link_property(Object *obj, const char *name,
--
2.0.0
[Qemu-devel] [PATCH memory v3 3/9] qom: Publish object_resolve_link
The lower level API object_resolve_path is already published to the
world as part of the QOM API. Add object_resolve link as well. This
allows QOM clients to roll their own link property setters without
having to fallback to the less safe object_resolve_path.
Signed-off-by: Peter Crosthwaite
---
include/qom/object.h | 16
qom/object.c | 13 ++---
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index a641dcd..7f96ecf 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1048,6 +1048,22 @@ Object *object_resolve_path_type(const char *path, const
char *typename,
Object *object_resolve_path_component(Object *parent, const gchar *part);
/**
+ * object_resolve_link:
+ * @obj: The object containing the link property
+ * @name: Name of the link property
+ * @path: the path to resolve
+ * @errp: Error object to populate in case of error
+ *
+ * Lookup an object and ensure its type matches a link property type. This
+ * is similar to object_resolve_path() except type verification against the
+ * link property is performed.
+ *
+ * Returns: The matched object or NULL on path lookup failures.
+ */
+Object *object_resolve_link(Object *obj, const char *name,
+const char *path, Error **errp);
+
+/**
* object_property_add_child:
* @obj: the object to add a property to
* @name: the name of the property
diff --git a/qom/object.c b/qom/object.c
index ec5adf4..b9b2736 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1058,17 +1058,8 @@ static void object_get_link_property(Object *obj,
Visitor *v, void *opaque,
}
}
-/*
- * object_resolve_link:
- *
- * Lookup an object and ensure its type matches the link property type. This
- * is similar to object_resolve_path() except type verification against the
- * link property is performed.
- *
- * Returns: The matched object or NULL on path lookup failures.
- */
-static Object *object_resolve_link(Object *obj, const char *name,
- const char *path, Error **errp)
+Object *object_resolve_link(Object *obj, const char *name,
+const char *path, Error **errp)
{
const char *type;
gchar *target_type;
--
2.0.0
[Qemu-devel] [PATCH memory v3 4/9] memory: Coreify subregion add functionality
Split off the core looping code that actually adds subregions into
it's own fn. This prepares support for Memory Region qomification
where setting the MR address or parent via QOM will back onto this more
minimal function.
Signed-off-by: Peter Crosthwaite
---
memory.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/memory.c b/memory.c
index 1352881..dd0a576 100644
--- a/memory.c
+++ b/memory.c
@@ -1410,18 +1410,15 @@ void memory_region_del_eventfd(MemoryRegion *mr,
memory_region_transaction_commit();
}
-static void memory_region_add_subregion_common(MemoryRegion *mr,
- hwaddr offset,
- MemoryRegion *subregion)
+static void do_memory_region_add_subregion_common(MemoryRegion *subregion)
{
+hwaddr offset = subregion->addr;
+MemoryRegion *mr = subregion->parent;
MemoryRegion *other;
memory_region_transaction_begin();
-assert(!subregion->parent);
memory_region_ref(subregion);
-subregion->parent = mr;
-subregion->addr = offset;
QTAILQ_FOREACH(other, &mr->subregions, subregions_link) {
if (subregion->may_overlap || other->may_overlap) {
continue;
@@ -1455,6 +1452,15 @@ done:
memory_region_transaction_commit();
}
+static void memory_region_add_subregion_common(MemoryRegion *mr,
+ hwaddr offset,
+ MemoryRegion *subregion)
+{
+assert(!subregion->parent);
+subregion->parent = mr;
+subregion->addr = offset;
+do_memory_region_add_subregion_common(subregion);
+}
void memory_region_add_subregion(MemoryRegion *mr,
hwaddr offset,
--
2.0.0
[Qemu-devel] [PATCH memory v3 6/9] memory: MemoryRegion: QOMify
QOMify memory regions as an Object. The former init() and destroy()
routines become instance_init() and instance_finalize() resp.
memory_region_init() is re-implemented to be:
object_initialize() + set fields
memory_region_destroy() is re-implemented to call finalize().
Signed-off-by: Peter Crosthwaite
---
changed since v2:
use object_unref() to Destroy MRs properly (Paolo Review)
Drop stale FIXME
include/exec/memory.h | 6 ++
memory.c | 53 +--
2 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..371c066 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -31,10 +31,15 @@
#include "qemu/queue.h"
#include "qemu/int128.h"
#include "qemu/notify.h"
+#include "qom/object.h"
#define MAX_PHYS_ADDR_SPACE_BITS 62
#define MAX_PHYS_ADDR(((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1)
+#define TYPE_MEMORY_REGION "qemu:memory-region"
+#define MEMORY_REGION(obj) \
+OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
+
typedef struct MemoryRegionOps MemoryRegionOps;
typedef struct MemoryRegionMmio MemoryRegionMmio;
@@ -130,6 +135,7 @@ typedef struct CoalescedMemoryRange CoalescedMemoryRange;
typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
struct MemoryRegion {
+Object parent_obj;
/* All fields are private - violators will be prosecuted */
const MemoryRegionOps *ops;
const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/memory.c b/memory.c
index aa25667..90c0557 100644
--- a/memory.c
+++ b/memory.c
@@ -850,35 +850,27 @@ void memory_region_init(MemoryRegion *mr,
const char *name,
uint64_t size)
{
-mr->ops = &unassigned_mem_ops;
-mr->opaque = NULL;
+object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
+
mr->owner = owner;
-mr->iommu_ops = NULL;
-mr->parent = NULL;
mr->size = int128_make64(size);
if (size == UINT64_MAX) {
mr->size = int128_2_64();
}
-mr->addr = 0;
-mr->subpage = false;
+mr->name = g_strdup(name);
+}
+
+static void memory_region_initfn(Object *obj)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+
+mr->ops = &unassigned_mem_ops;
mr->enabled = true;
-mr->terminates = false;
-mr->ram = false;
mr->romd_mode = true;
-mr->readonly = false;
-mr->rom_device = false;
mr->destructor = memory_region_destructor_none;
-mr->priority = 0;
-mr->may_overlap = false;
-mr->alias = NULL;
QTAILQ_INIT(&mr->subregions);
memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
QTAILQ_INIT(&mr->coalesced);
-mr->name = g_strdup(name);
-mr->dirty_log_mask = 0;
-mr->ioeventfd_nb = 0;
-mr->ioeventfds = NULL;
-mr->flush_coalesced_mmio = false;
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1099,8 +1091,10 @@ void memory_region_init_reservation(MemoryRegion *mr,
memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size);
}
-void memory_region_destroy(MemoryRegion *mr)
+static void memory_region_finalize(Object *obj)
{
+MemoryRegion *mr = MEMORY_REGION(obj);
+
assert(QTAILQ_EMPTY(&mr->subregions));
assert(memory_region_transaction_depth == 0);
mr->destructor(mr);
@@ -1109,6 +1103,12 @@ void memory_region_destroy(MemoryRegion *mr)
g_free(mr->ioeventfds);
}
+void memory_region_destroy(MemoryRegion *mr)
+{
+object_unref(OBJECT(mr));
+}
+
+
Object *memory_region_owner(MemoryRegion *mr)
{
return mr->owner;
@@ -1886,3 +1886,18 @@ void mtree_info(fprintf_function mon_printf, void *f)
g_free(ml);
}
}
+
+static const TypeInfo memory_region_info = {
+.parent = TYPE_OBJECT,
+.name = TYPE_MEMORY_REGION,
+.instance_size = sizeof(MemoryRegion),
+.instance_init = memory_region_initfn,
+.instance_finalize = memory_region_finalize,
+};
+
+static void memory_register_types(void)
+{
+type_register_static(&memory_region_info);
+}
+
+type_init(memory_register_types)
--
2.0.0
[Qemu-devel] [PATCH memory v3 5/9] memory: MemoryRegion: factor out memory region re-adder
memory_region_set_address is mostly just a function that deletes and
re-adds a memory region. Factor this generic functionality out into a
re-usable function. This prepares support for further QOMification
of MemoryRegion.
Signed-off-by: Peter Crosthwaite
---
memory.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/memory.c b/memory.c
index dd0a576..aa25667 100644
--- a/memory.c
+++ b/memory.c
@@ -828,6 +828,23 @@ static void
memory_region_destructor_rom_device(MemoryRegion *mr)
qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
}
+static void do_memory_region_add_subregion_common(MemoryRegion *subregion);
+
+static void memory_region_readd_subregion(MemoryRegion *mr)
+{
+MemoryRegion *parent = mr->parent;
+
+if (parent) {
+memory_region_transaction_begin();
+memory_region_ref(mr);
+memory_region_del_subregion(parent, mr);
+mr->parent = parent;
+do_memory_region_add_subregion_common(mr);
+memory_region_unref(mr);
+memory_region_transaction_commit();
+}
+}
+
void memory_region_init(MemoryRegion *mr,
Object *owner,
const char *name,
@@ -1506,19 +1523,10 @@ void memory_region_set_enabled(MemoryRegion *mr, bool
enabled)
void memory_region_set_address(MemoryRegion *mr, hwaddr addr)
{
-MemoryRegion *parent = mr->parent;
-
-if (addr == mr->addr || !parent) {
+if (addr != mr->addr) {
mr->addr = addr;
-return;
+memory_region_readd_subregion(mr);
}
-
-memory_region_transaction_begin();
-memory_region_ref(mr);
-memory_region_del_subregion(parent, mr);
-memory_region_add_subregion_common(parent, addr, mr);
-memory_region_unref(mr);
-memory_region_transaction_commit();
}
void memory_region_set_alias_offset(MemoryRegion *mr, hwaddr offset)
--
2.0.0
[Qemu-devel] [PATCH memory v3 7/9] memory: MemoryRegion: Add container and addr props
Expose the already existing .parent and .addr fields as QOM properties.
.parent (i.e. the field describing the memory region that contains this
one in Memory hierachy) is renamed "container". This is to avoid
confusion with the owner field, which is much more akin to an actual QOM
parent.
Setting the .parent ("container") will cause the memory subregion adding
to happen. Nullifying or changing the .parent will delete or relocate
(to a different container) the subregion resp.
Setting or changing the address will relocate the memory region.
Signed-off-by: Peter Crosthwaite
---
changed since v2:
Removed un-needed strdup
changed since v1:
Converted container to low level link property and added subregion setup.
memory.c | 105 +++
1 file changed, 105 insertions(+)
diff --git a/memory.c b/memory.c
index 90c0557..0296675 100644
--- a/memory.c
+++ b/memory.c
@@ -16,6 +16,7 @@
#include "exec/memory.h"
#include "exec/address-spaces.h"
#include "exec/ioport.h"
+#include "qapi/visitor.h"
#include "qemu/bitops.h"
#include "qom/object.h"
#include "trace.h"
@@ -860,6 +861,99 @@ void memory_region_init(MemoryRegion *mr,
mr->name = g_strdup(name);
}
+static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+uint64_t value = mr->addr;
+
+visit_type_uint64(v, &value, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+}
+}
+
+static void memory_region_set_addr(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+uint64_t value;
+
+visit_type_uint64(v, &value, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+memory_region_set_address(mr, value);
+}
+
+static void memory_region_set_container(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+MemoryRegion *old_parent = mr->parent;
+MemoryRegion *new_parent = NULL;
+char *path = NULL;
+
+visit_type_str(v, &path, name, &local_err);
+
+if (!local_err && strcmp(path, "") != 0) {
+new_parent = MEMORY_REGION(object_resolve_link(obj, name, path,
+ &local_err));
+}
+
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+object_ref(OBJECT(new_parent));
+
+memory_region_transaction_begin();
+memory_region_ref(mr);
+if (old_parent) {
+memory_region_del_subregion(old_parent, mr);
+}
+mr->parent = new_parent;
+if (new_parent) {
+do_memory_region_add_subregion_common(mr);
+}
+memory_region_unref(mr);
+memory_region_transaction_commit();
+
+object_unref(OBJECT(old_parent));
+}
+
+static void memory_region_get_container(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+gchar *path = (gchar *)"";
+
+if (mr->parent) {
+path = object_get_canonical_path(OBJECT(mr->parent));
+}
+visit_type_str(v, &path, name, errp);
+if (mr->parent) {
+g_free(path);
+}
+}
+
+static void memory_region_release_container(Object *obj, const char *name,
+void *opaque)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+
+if (mr->parent) {
+memory_region_del_subregion(mr->parent, mr);
+object_unref(OBJECT(mr->parent));
+}
+}
+
static void memory_region_initfn(Object *obj)
{
MemoryRegion *mr = MEMORY_REGION(obj);
@@ -871,6 +965,17 @@ static void memory_region_initfn(Object *obj)
QTAILQ_INIT(&mr->subregions);
memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
QTAILQ_INIT(&mr->coalesced);
+
+object_property_add(OBJECT(mr), "container", "link<" TYPE_MEMORY_REGION
">",
+memory_region_get_container,
+memory_region_set_container,
+memory_region_release_container,
+NULL, &error_abort);
+
+object_property_add(OBJECT(mr), "addr", "uint64",
+memory_region_get_addr,
+memory_region_set_addr,
+NULL, NULL, &error_abort);
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
--
2.0.0
[Qemu-devel] [PATCH memory v3 8/9] memory: MemoryRegion: Add may-overlap and priority props
QOM propertyify the .may-overlap and .priority fields. The setters
will re-add the memory as a subregion if needed (i.e. the values change
when the memory region is already contained).
Signed-off-by: Peter Crosthwaite
---
changed since v2:
Fixed priority getter
Support signed values in priority set/get
changed since v1:
Converted priority to signed type
include/exec/memory.h | 2 +-
memory.c | 57 +++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 371c066..117c0d3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -157,7 +157,7 @@ struct MemoryRegion {
bool flush_coalesced_mmio;
MemoryRegion *alias;
hwaddr alias_offset;
-int priority;
+int32_t priority;
bool may_overlap;
QTAILQ_HEAD(subregions, MemoryRegion) subregions;
QTAILQ_ENTRY(MemoryRegion) subregions_link;
diff --git a/memory.c b/memory.c
index 0296675..2d98020 100644
--- a/memory.c
+++ b/memory.c
@@ -954,6 +954,55 @@ static void memory_region_release_container(Object *obj,
const char *name,
}
}
+static void memory_region_get_priority(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+int32_t value = mr->priority;
+
+visit_type_int32(v, &value, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+}
+}
+
+static void memory_region_set_priority(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+int32_t value;
+
+visit_type_int32(v, &value, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+if (mr->priority != value) {
+mr->priority = value;
+memory_region_readd_subregion(mr);
+}
+}
+
+static bool memory_region_get_may_overlap(Object *obj, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+
+return mr->may_overlap;
+}
+
+static void memory_region_set_may_overlap(Object *obj, bool value, Error
**errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+
+if (mr->may_overlap != value) {
+mr->may_overlap = value;
+memory_region_readd_subregion(mr);
+}
+}
+
static void memory_region_initfn(Object *obj)
{
MemoryRegion *mr = MEMORY_REGION(obj);
@@ -976,6 +1025,14 @@ static void memory_region_initfn(Object *obj)
memory_region_get_addr,
memory_region_set_addr,
NULL, NULL, &error_abort);
+object_property_add(OBJECT(mr), "priority", "uint32",
+memory_region_get_priority,
+memory_region_set_priority,
+NULL, NULL, &error_abort);
+object_property_add_bool(OBJECT(mr), "may-overlap",
+ memory_region_get_may_overlap,
+ memory_region_set_may_overlap,
+ &error_abort);
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
--
2.0.0
[Qemu-devel] [PATCH memory v3 9/9] memory: MemoryRegion: Add size property
To allow devices to dynamically resize the device. The motivation is
to allow devices with variable size to init their memory_region
without size early and then correctly populate size at realize() time.
Signed-off-by: Peter Crosthwaite
---
memory.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/memory.c b/memory.c
index 2d98020..54fdbb2 100644
--- a/memory.c
+++ b/memory.c
@@ -1003,6 +1003,40 @@ static void memory_region_set_may_overlap(Object *obj,
bool value, Error **errp)
}
}
+static void memory_region_get_size(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+uint64_t value = int128_get64(mr->size);
+
+visit_type_uint64(v, &value, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+}
+}
+
+static void memory_region_set_size(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+Error *local_err = NULL;
+uint64_t value;
+Int128 v128;
+
+visit_type_uint64(v, &value, name, &local_err);
+v128 = int128_make64(value);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+if (!int128_eq(v128, mr->size)) {
+mr->size = v128;
+memory_region_readd_subregion(mr);
+}
+}
+
static void memory_region_initfn(Object *obj)
{
MemoryRegion *mr = MEMORY_REGION(obj);
@@ -1033,6 +1067,10 @@ static void memory_region_initfn(Object *obj)
memory_region_get_may_overlap,
memory_region_set_may_overlap,
&error_abort);
+object_property_add(OBJECT(mr), "size", "uint64",
+memory_region_get_size,
+memory_region_set_size,
+NULL, NULL, &error_abort);
}
static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
--
2.0.0
Re: [Qemu-devel] [PATCH 3/3] block: Drop some superfluous casts from void *
Max Reitz writes: > On 30.05.2014 18:30, Markus Armbruster wrote: >> They clutter the code. Unfortunately, I can't figure out how to make >> Coccinelle drop all of them, so I have to settle for common special >> cases: >> >> @@ >> type T; >> T *pt; >> void *pv; >> @@ >> - pt = (T *)pv; >> + pt = pv; >> @@ >> type T; >> @@ >> - (T *) >>(\(g_malloc\|g_malloc0\|g_realloc\|g_new\|g_new0\|g_renew\| >> g_try_malloc\|g_try_malloc0\|g_try_realloc\| >> g_try_new\|g_try_new0\|g_try_renew\)(...)) >> >> Signed-off-by: Markus Armbruster >> --- >> block/sheepdog.c| 2 +- >> block/vhdx-log.c| 2 +- >> block/vvfat.c | 8 >> hw/ide/microdrive.c | 2 +- >> 4 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/block/sheepdog.c b/block/sheepdog.c >> index 0709fd0..340bdf6 100644 >> --- a/block/sheepdog.c >> +++ b/block/sheepdog.c >> @@ -2202,7 +2202,7 @@ static int sd_snapshot_create(BlockDriverState *bs, >> QEMUSnapshotInfo *sn_info) >> goto cleanup; >> } >> -inode = (SheepdogInode *)g_malloc(datalen); >> +inode = g_malloc(datalen); >> ret = read_object(fd, (char *)inode, >> vid_to_vdi_oid(new_vid), >> s->inode.nr_copies, datalen, 0, s->cache_flags); >> diff --git a/block/vhdx-log.c b/block/vhdx-log.c >> index 3eb7e68..f07328d 100644 >> --- a/block/vhdx-log.c >> +++ b/block/vhdx-log.c >> @@ -909,7 +909,7 @@ static int vhdx_log_write(BlockDriverState *bs, >> BDRVVHDXState *s, >> buffer = qemu_blockalign(bs, total_length); >> memcpy(buffer, &new_hdr, sizeof(new_hdr)); >> -new_desc = (VHDXLogDescriptor *) (buffer + sizeof(new_hdr)); >> +new_desc = buffer + sizeof(new_hdr); > > Perhaps we should try to fix void pointer arithmetic some time... Fixes for that tend to involve back-and-forth type casting. I find that cure not much of an improvement over the disease :) > Anyway: > > Reviewed-by: Max Reitz Thanks!
Re: [Qemu-devel] [PATCH] qemu-img: Report error even with --oformat=json
Max Reitz writes:
> img_check() should report that the format of the given image does not
> support checks even if JSON output is desired. JSON data is output to
> stdout, as opposed to error messages, which are (in the case of
> qemu-img) printed to stderr. Therefore, it is easy to distinguish
> between the two.
>
> Also, img_info() does already use error_report() for human-readable
> messages even though JSON output is desired (through
> collect_image_info_list()).
>
> Signed-off-by: Max Reitz
> ---
> As the old code deliberately emitted the error only in case of
> human-readable output, I asked Federico, the original author, for his
> opinion in https://bugzilla.redhat.com/show_bug.cgi?id=1054753, but did
> not receive a reply so far.
> ---
> qemu-img.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index d118da5..b3d2bc6 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -663,9 +663,7 @@ static int img_check(int argc, char **argv)
> ret = collect_image_check(bs, check, filename, fmt, fix);
>
> if (ret == -ENOTSUP) {
> -if (output_format == OFORMAT_HUMAN) {
> -error_report("This image format does not support checks");
> -}
> +error_report("This image format does not support checks");
> ret = 63;
Undocumented return code. Care to fix that, too?
> goto fail;
> }
Reviewed-by: Markus Armbruster
[Qemu-devel] [PATCH v2] trace: Replace fprintf with error_report and print location
This replaces fprintf(stderr) with error_report.
This moves local variables to the beginning of the function to comply
with QEMU's coding style.
Suggested-by: Lluís Vilanova
Signed-off-by: Alexey Kardashevskiy
---
Changes:
v2:
* polished commit log
---
trace/control.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/trace/control.c b/trace/control.c
index 4aa02cf..4ee2bd2 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -8,7 +8,7 @@
*/
#include "trace/control.h"
-
+#include "qemu/error-report.h"
TraceEvent *trace_event_name(const char *name)
{
@@ -81,18 +81,24 @@ TraceEvent *trace_event_pattern(const char *pat, TraceEvent
*ev)
void trace_backend_init_events(const char *fname)
{
+Location loc;
+FILE *fp;
+char line_buf[1024];
+size_t line_idx = 0;
+
if (fname == NULL) {
return;
}
-FILE *fp = fopen(fname, "r");
+loc_push_none(&loc);
+loc_set_file(fname, 0);
+fp = fopen(fname, "r");
if (!fp) {
-fprintf(stderr, "error: could not open trace events file '%s': %s\n",
-fname, strerror(errno));
+error_report("%s", strerror(errno));
exit(1);
}
-char line_buf[1024];
while (fgets(line_buf, sizeof(line_buf), fp)) {
+loc_set_file(fname, ++line_idx);
size_t len = strlen(line_buf);
if (len > 1) { /* skip empty lines */
line_buf[len - 1] = '\0';
@@ -111,13 +117,11 @@ void trace_backend_init_events(const char *fname)
} else {
TraceEvent *ev = trace_event_name(line_ptr);
if (ev == NULL) {
-fprintf(stderr,
-"WARNING: trace event '%s' does not exist\n",
-line_ptr);
+error_report("WARNING: trace event '%s' does not exist",
+ line_ptr);
} else if (!trace_event_get_state_static(ev)) {
-fprintf(stderr,
-"WARNING: trace event '%s' is not traceable\n",
-line_ptr);
+error_report("WARNING: trace event '%s' is not
traceable\n",
+ line_ptr);
} else {
trace_event_set_state_dynamic(ev, enable);
}
@@ -125,8 +129,9 @@ void trace_backend_init_events(const char *fname)
}
}
if (fclose(fp) != 0) {
-fprintf(stderr, "error: closing file '%s': %s\n",
-fname, strerror(errno));
+loc_set_file(fname, 0);
+error_report("%s", strerror(errno));
exit(1);
}
+loc_pop(&loc);
}
--
2.0.0
