Xen devel - Apr 2011 - vmport patch for run Fedora 14 [PATCH V14 00/17] Xen device model support

Anthony and upstream-qemu testers,

I tried to run upstream-qemu with v14 patch set and found Fedora 14
not bootable on HVM domain.
Now it''s running with vmport workaround patch at bottom of this mail.


vmport device enables guests to know some configurations of the
virtual machine itself. Actually it is not mandatory run guests,
so disabled it to avoid qemu crash.

I am not sure whether this fix is best way or not.
In my thought it is ideal to take one of these solutions, but I could not.

1) fix vmport so that it works on Xen.

I need to read vCPU registers, however I could not figure out
how to do that on Xen. ;-)

2) disable vmport device entirely.

I found vmmouse expects initialization of vmport,
therefore additional patch will be needed.


This may be  a side-effect of the change below.
> > > +        /* Xen require only one Qemu VCPU */
> > >          pc_new_cpu(cpu_model);
> >
> > This looks a bit fishy. What is the semantic of -smp 2 or more in Xen
> > mode? If that is an invalid/unused configuration option, catch that
and
> > reject it instead of installing this workaround. If it has a valid
> > semantic, please elaborate why you need to restrict the number of
> > instantiated cpus. Just to optimize memory usage?
>
> I thought in a first place that was needed to avoid errors. But it works
> also when we initialise other CPUs. But I prefere to keep it that way to
> save memory and in the case where there is a thread for each cpu that
> will also avoid to have many useless threads.
>
> Also, I use -smp i to initialise the xen''s structures related to
the
> vcpu.
Thanks,
Takeshi


diff --git a/hw/vmport.c b/hw/vmport.c
index 19010e4..316cf1d 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -61,6 +61,9 @@ static uint32_t vmport_ioport_read(void *opaque,
uint32_t addr)
     unsigned char command;
     uint32_t eax;

+    if (env == NULL)
+        return 0;
+
     cpu_synchronize_state(env);

     eax = env->regs[R_EAX];
@@ -85,6 +88,9 @@ static void vmport_ioport_write(void *opaque,
uint32_t addr, uint32_t val)
 {
     CPUState *env = cpu_single_env;

+    if (env == NULL)
+        return;
+
     env->regs[R_EAX] = vmport_ioport_read(opaque, addr);
 }


2011/4/21  <anthony.perard@citrix.com>:
> From: Anthony PERARD <anthony.perard@citrix.com>
>
> Hi all,
>
> Update of the patch series that address comment from Jan Kiszka.
>
> The change v13->v14:
>  - Remove of ram_size parameter from pc_memory_init
>  - set both below/above_4g_mem_size at the same place in the code.
>
> Change v12->v13:
>  - There are few changes in the xen init code. A xen_hvm_init function is
new
>    in this patch set and is call from xenfv:machine->init.
>  -> So "-xen-create -M xenpv" will continue to work as before
this patch
>    series.
>  - There is a new reset handler to set env->halted = 0 on the first
vcpu.
>  - One change have been made to pc_memory_init, the calculation of
>    below/above_4g_mem_size have been moved to pc_init1. This is to remove a
>    "random" "if (xen()) return;" in pc_memory_init.
>  - xen_map_block is a new function to map RAMBlock that belong to a ROM/RAM
of
>    a device.
>  - fix cpu_physical_memory_unmap with mapcache, Because qemu_get_ram_ptr
can
>    be called more than one time in cpu_physical_memory_map,
qemu_put_ram_ptr
>    need to be called the same amount of time.
>  - Add some trace_* call.
>
>
> This series depends on the series "Introduce "machine"
QemuOpts".
>
> You can find a git tree here:
>
> git://xenbits.xen.org/people/aperard/qemu-dm.git qemu-dm-v14
>
>
>
> Anthony PERARD (13):
>  xen: Replace some tab-indents with spaces (clean-up).
>  xen: Make Xen build once.
>  xen: Support new libxc calls from xen unstable.
>  xen: Add initialisation of Xen
>  pc_memory_init: Move memory calculation to the caller.
>  xen: Add xenfv machine
>  piix_pci: Introduces Xen specific call for irq.
>  xen: Introduce Xen Interrupt Controller
>  Introduce qemu_put_ram_ptr
>  configure: Always use 64bits target physical addresses with xen
>    enabled.
>  vl.c: Introduce getter for shutdown_requested and reset_requested.
>  xen: Set running state in xenstore.
>  xen: Add Xen hypercall for sleep state in the cmos_s3 callback.
>
> Arun Sharma (1):
>  xen: Initialize event channels and io rings
>
> John Baboval (2):
>  xen: Adds a cap to the number of map cache entries.
>  pci: Use of qemu_put_ram_ptr in pci_add_option_rom.
>
> Jun Nakajima (1):
>  xen: Introduce the Xen mapcache
>
>  Makefile.target      |   14 +-
>  configure            |   71 ++++++-
>  cpu-common.h         |    1 +
>  exec.c               |   86 +++++++-
>  hw/pc.c              |   17 +--
>  hw/pc.h              |    8 +-
>  hw/pc_piix.c         |   69 ++++++-
>  hw/pci.c             |    2 +
>  hw/piix_pci.c        |   47 ++++-
>  hw/xen.h             |   41 ++++
>  hw/xen_backend.c     |  421 +++++++++++++++++++----------------
>  hw/xen_backend.h     |    6 +-
>  hw/xen_common.h      |  106 ++++++++--
>  hw/xen_disk.c        |  496 ++++++++++++++++++++++-------------------
>  hw/xen_domainbuild.c |    3 +-
>  hw/xen_machine_pv.c  |    1 +
>  hw/xen_nic.c         |  265 ++++++++++++----------
>  sysemu.h             |    2 +
>  trace-events         |   13 +
>  vl.c                 |   12 +
>  xen-all.c            |  605
++++++++++++++++++++++++++++++++++++++++++++++++++
>  xen-mapcache-stub.c  |   44 ++++
>  xen-mapcache.c       |  375 +++++++++++++++++++++++++++++++
>  xen-mapcache.h       |   37 +++
>  xen-stub.c           |   41 ++++
>  25 files changed, 2187 insertions(+), 596 deletions(-)
>  create mode 100644 xen-all.c
>  create mode 100644 xen-mapcache-stub.c
>  create mode 100644 xen-mapcache.c
>  create mode 100644 xen-mapcache.h
>  create mode 100644 xen-stub.c
>
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


-- 
Takeshi HASEGAWA <hasegaw@gmail.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

On Tue, 26 Apr 2011, Takeshi HASEGAWA wrote:
> Anthony and upstream-qemu testers,
>
> I tried to run upstream-qemu with v14 patch set and found Fedora 14
> not bootable on HVM domain.
> Now it''s running with vmport workaround patch at bottom of this
mail.
>
>
> vmport device enables guests to know some configurations of the
> virtual machine itself. Actually it is not mandatory run guests,
> so disabled it to avoid qemu crash.
>
> I am not sure whether this fix is best way or not.
> In my thought it is ideal to take one of these solutions, but I could not.
>
> 1) fix vmport so that it works on Xen.
>
> I need to read vCPU registers, however I could not figure out
> how to do that on Xen. ;-)
>
> 2) disable vmport device entirely.
I think it''s better to disable vmport with Xen, because we will not
sync
the vcpu registers, at least in this patch series.

So I will just add the patch below to the patch series.


diff --git a/hw/pc.c b/hw/pc.c
index ebdf3b0..8106197 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1082,7 +1082,8 @@ static void cpu_request_exit(void *opaque, int irq, int
level)
 }

 void pc_basic_device_init(qemu_irq *isa_irq,
-                          ISADevice **rtc_state)
+                          ISADevice **rtc_state,
+                          bool no_vmport)
 {
     int i;
     DriveInfo *fd[MAX_FD];
@@ -1127,8 +1128,12 @@ void pc_basic_device_init(qemu_irq *isa_irq,
     a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
     i8042 = isa_create_simple("i8042");
     i8042_setup_a20_line(i8042, &a20_line[0]);
-    vmport_init();
-    vmmouse = isa_try_create("vmmouse");
+    if (!no_vmport) {
+        vmport_init();
+        vmmouse = isa_try_create("vmmouse");
+    } else {
+        vmmouse = NULL;
+    }
     if (vmmouse) {
         qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse", i8042);
         qdev_init_nofail(&vmmouse->qdev);
diff --git a/hw/pc.h b/hw/pc.h
index cc7ba58..0dcbee7 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -137,7 +137,8 @@ void pc_memory_init(const char *kernel_filename,
 qemu_irq *pc_allocate_cpu_irq(void);
 void pc_vga_init(PCIBus *pci_bus);
 void pc_basic_device_init(qemu_irq *isa_irq,
-                          ISADevice **rtc_state);
+                          ISADevice **rtc_state,
+                          bool no_vmport);
 void pc_init_ne2k_isa(NICInfo *nd);
 void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
                   const char *boot_device,
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 4ff4a55..9a22a8a 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -141,7 +141,7 @@ static void pc_init1(ram_addr_t ram_size,
     pc_vga_init(pci_enabled? pci_bus: NULL);

     /* init basic PC hardware */
-    pc_basic_device_init(isa_irq, &rtc_state);
+    pc_basic_device_init(isa_irq, &rtc_state, xen_enabled());

     for(i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];


> I found vmmouse expects initialization of vmport,
> therefore additional patch will be needed.
>
>
> This may be  a side-effect of the change below.
>
> > > > +        /* Xen require only one Qemu VCPU */
> > > >          pc_new_cpu(cpu_model);
> > >
> > > This looks a bit fishy. What is the semantic of -smp 2 or more in
Xen
> > > mode? If that is an invalid/unused configuration option, catch
that and
> > > reject it instead of installing this workaround. If it has a
valid
> > > semantic, please elaborate why you need to restrict the number of
> > > instantiated cpus. Just to optimize memory usage?
> >
> > I thought in a first place that was needed to avoid errors. But it
works
> > also when we initialise other CPUs. But I prefere to keep it that way
to
> > save memory and in the case where there is a thread for each cpu that
> > will also avoid to have many useless threads.
> >
> > Also, I use -smp i to initialise the xen''s structures related
to the
> > vcpu.
>
Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Anthony,

Thanks for the work!
I will inform when I found something else.

Thanks,
Takeshi

2011/5/4 Anthony PERARD
<anthony.perard@citrix.com>:
> Hi,
>
> On Tue, 26 Apr 2011, Takeshi HASEGAWA wrote:
>
>> Anthony and upstream-qemu testers,
>>
>> I tried to run upstream-qemu with v14 patch set and found Fedora 14
>> not bootable on HVM domain.
>> Now it''s running with vmport workaround patch at bottom of
this mail.
>>
>>
>> vmport device enables guests to know some configurations of the
>> virtual machine itself. Actually it is not mandatory run guests,
>> so disabled it to avoid qemu crash.
>>
>> I am not sure whether this fix is best way or not.
>> In my thought it is ideal to take one of these solutions, but I could
not.
>>
>> 1) fix vmport so that it works on Xen.
>>
>> I need to read vCPU registers, however I could not figure out
>> how to do that on Xen. ;-)
>>
>> 2) disable vmport device entirely.
>
> I think it''s better to disable vmport with Xen, because we will
not sync
> the vcpu registers, at least in this patch series.
>
> So I will just add the patch below to the patch series.
>
>
> diff --git a/hw/pc.c b/hw/pc.c
> index ebdf3b0..8106197 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1082,7 +1082,8 @@ static void cpu_request_exit(void *opaque, int irq,
int level)
>  }
>
>  void pc_basic_device_init(qemu_irq *isa_irq,
> -                          ISADevice **rtc_state)
> +                          ISADevice **rtc_state,
> +                          bool no_vmport)
>  {
>     int i;
>     DriveInfo *fd[MAX_FD];
> @@ -1127,8 +1128,12 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>     a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
>     i8042 = isa_create_simple("i8042");
>     i8042_setup_a20_line(i8042, &a20_line[0]);
> -    vmport_init();
> -    vmmouse = isa_try_create("vmmouse");
> +    if (!no_vmport) {
> +        vmport_init();
> +        vmmouse = isa_try_create("vmmouse");
> +    } else {
> +        vmmouse = NULL;
> +    }
>     if (vmmouse) {
>         qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse",
i8042);
>         qdev_init_nofail(&vmmouse->qdev);
> diff --git a/hw/pc.h b/hw/pc.h
> index cc7ba58..0dcbee7 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -137,7 +137,8 @@ void pc_memory_init(const char *kernel_filename,
>  qemu_irq *pc_allocate_cpu_irq(void);
>  void pc_vga_init(PCIBus *pci_bus);
>  void pc_basic_device_init(qemu_irq *isa_irq,
> -                          ISADevice **rtc_state);
> +                          ISADevice **rtc_state,
> +                          bool no_vmport);
>  void pc_init_ne2k_isa(NICInfo *nd);
>  void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>                   const char *boot_device,
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 4ff4a55..9a22a8a 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -141,7 +141,7 @@ static void pc_init1(ram_addr_t ram_size,
>     pc_vga_init(pci_enabled? pci_bus: NULL);
>
>     /* init basic PC hardware */
> -    pc_basic_device_init(isa_irq, &rtc_state);
> +    pc_basic_device_init(isa_irq, &rtc_state, xen_enabled());
>
>     for(i = 0; i < nb_nics; i++) {
>         NICInfo *nd = &nd_table[i];
>
>
>
>> I found vmmouse expects initialization of vmport,
>> therefore additional patch will be needed.
>>
>>
>> This may be  a side-effect of the change below.
>>
>> > > > +        /* Xen require only one Qemu VCPU */
>> > > >          pc_new_cpu(cpu_model);
>> > >
>> > > This looks a bit fishy. What is the semantic of -smp 2 or
more in Xen
>> > > mode? If that is an invalid/unused configuration option,
catch that and
>> > > reject it instead of installing this workaround. If it has a
valid
>> > > semantic, please elaborate why you need to restrict the
number of
>> > > instantiated cpus. Just to optimize memory usage?
>> >
>> > I thought in a first place that was needed to avoid errors. But it
works
>> > also when we initialise other CPUs. But I prefere to keep it that
way to
>> > save memory and in the case where there is a thread for each cpu
that
>> > will also avoid to have many useless threads.
>> >
>> > Also, I use -smp i to initialise the xen''s structures
related to the
>> > vcpu.
>>
>
> Thanks,
>
> --
> Anthony PERARD
>


-- 
Takeshi HASEGAWA <hasegaw@gmail.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel