Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] spapr: compute interrupt vector address from LPCR
On Sun, Apr 03, 2016 at 07:57:50PM +0200, Cédric Le Goater wrote:
> On 04/01/2016 04:43 AM, David Gibson wrote:
> > On Thu, Mar 31, 2016 at 08:13:09AM +0100, Mark Cave-Ayland wrote:
> >> On 31/03/16 05:55, David Gibson wrote:
> >>
> >>> On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote:
> This address is changed by the linux kernel using the H_SET_MODE hcall
> and needs to be restored when migrating a spapr VM running in
> TCG. This can be done using the AIL bits from the LPCR register.
>
> The patch introduces a helper routine cpu_ppc_get_excp_prefix() which
> returns the effective address offset of the interrupt handler
> depending on the LPCR_AIL bits. The same helper can be used in the
> H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_*
> defines.
>
> Signed-off-by: Cédric Le Goater
> >>>
> >>> I've applied this (with Greg's minor amendments) to ppc-for-2.6),
> >>> since it certainly improves behaviour, although I have a couple of
> >>> queries:
> >>>
> ---
>
> Changes since v1:
>
> - moved helper routine under target-ppc/
> - moved the restore of excp_prefix under cpu_post_load()
>
> hw/ppc/spapr_hcall.c| 13 ++---
> include/hw/ppc/spapr.h |5 -
> target-ppc/cpu.h|9 +
> target-ppc/machine.c| 20 +++-
> target-ppc/translate_init.c | 14 ++
> 5 files changed, 44 insertions(+), 17 deletions(-)
>
> Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c
> ===
> --- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c
> +++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c
> @@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_
> return H_P4;
> }
>
> -switch (mflags) {
> -case H_SET_MODE_ADDR_TRANS_NONE:
> -prefix = 0;
> -break;
> -case H_SET_MODE_ADDR_TRANS_0001_8000:
> -prefix = 0x18000;
> -break;
> -case H_SET_MODE_ADDR_TRANS_C000___4000:
> -prefix = 0xC0004000ULL;
> -break;
> -default:
> +prefix = cpu_ppc_get_excp_prefix(mflags);
> +if (prefix == (target_ulong) -1ULL) {
> return H_UNSUPPORTED_FLAG;
> }
>
> Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
> ===
> --- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
> +++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
> @@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque)
> }
> }
>
> +
> +static int cpu_post_load_excp_prefix(CPUPPCState *env)
> +{
> +int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> +target_ulong prefix = cpu_ppc_get_excp_prefix(ail);
> +
> +if (prefix == (target_ulong) -1ULL) {
> +return -EINVAL;
> +}
> +env->excp_prefix = prefix;
> +return 0;
> +}
> +
> static int cpu_post_load(void *opaque, int version_id)
> {
> PowerPCCPU *cpu = opaque;
> CPUPPCState *env = &cpu->env;
> int i;
> target_ulong msr;
> +int ret = 0;
>
> /*
> * We always ignore the source PVR. The user or management
> @@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i
>
> hreg_compute_mem_idx(env);
>
> -return 0;
> +if (env->spr[SPR_LPCR] & LPCR_AIL) {
> +ret = cpu_post_load_excp_prefix(env);
> +}
> >>>
> >>> Why not call this unconditionally? If AIL == 0 it will still do the
> >>> right thing.
> >>>
> >>> Aren't there also circumstances where the exception prefix can depend
> >>> on the MSR? Do those need to be handled somewhere?
> >>
> >> Yes indeed - this was part of my patchset last year to fix up various
> >> migration issues for the Mac PPC machines (see commit
> >> 2360b6e84f78d41fa0f76555a947148b73645259).
> >>
> >> I agree that having the env->excp_prefix logic split like this isn't a
> >> particularly great idea. Let's just have a single helper function as in
> >> the patch above and use that in both places (and in fact with recent
> >> changes I have a feeling env->excp_prefix is one of the few remaining
> >> reasons that the above change is still required, but please check this).
> >
> > Right.
> >
> > So, what I'd really prefer to see here is a single
> > update_excp_prefix() helper which will set env->excp_prefix based on
> > whatever registers are relevant (LPCR, MSR and probably the cpu
> > model). That would be called on incoming migration, and after
> > updating any of the relevant registers (so fro
Re: [Qemu-devel] [RFC PATCH v2.1 12/12] spapr: CPU hot unplug support
On Thu, Mar 31, 2016 at 02:09:21PM +0530, Bharata B Rao wrote:
> Remove the CPU core device by removing the underlying CPU thread devices.
> Hot removal of CPU for sPAPR guests is achieved by sending the hot unplug
> notification to the guest. Release the vCPU object after CPU hot unplug so
> that vCPU fd can be parked and reused.
>
> Signed-off-by: Bharata B Rao
> ---
> hw/ppc/spapr.c | 16
> hw/ppc/spapr_cpu_core.c | 86
> +
> include/hw/ppc/spapr.h | 1 +
> include/hw/ppc/spapr_cpu_core.h | 11 ++
> 4 files changed, 114 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1a5dbd9..74cdcf2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2348,11 +2348,27 @@ static void spapr_machine_device_plug(HotplugHandler
> *hotplug_dev,
> }
> }
>
> +void spapr_cpu_destroy(PowerPCCPU *cpu)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +
> +xics_cpu_destroy(spapr->icp, cpu);
> +qemu_unregister_reset(spapr_cpu_reset, cpu);
> +}
> +
> static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
> +
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> error_setg(errp, "Memory hot unplug not supported by sPAPR");
> +} else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
> +if (!smc->dr_cpu_enabled) {
> +error_setg(errp, "CPU hot unplug not supported on this machine");
> +return;
> +}
> +spapr_core_unplug(hotplug_dev, dev, errp);
> }
> }
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index a9ba843..09a592e 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -119,6 +119,92 @@ void spapr_core_plug(HotplugHandler *hotplug_dev,
> DeviceState *dev,
> }
> }
>
> +static void spapr_cpu_core_cleanup(struct sPAPRCPUUnplugList *unplug_list)
> +{
> +sPAPRCPUUnplug *unplug, *next;
> +Object *cpu;
> +
> +QLIST_FOREACH_SAFE(unplug, unplug_list, node, next) {
> +cpu = unplug->cpu;
> +object_unparent(cpu);
Is there any danger in the fact that the cpu object is still in the
QOM tree until unparented here? My usual expectation would be that
you'd remove the object from the tree immediately, but defer the
actual free. But I'm a bit unclear on how QOM removals are supposed
to work.
> +QLIST_REMOVE(unplug, node);
> +g_free(unplug);
> +}
> +}
> +
> +static void spapr_add_cpu_to_unplug_list(Object *cpu,
> + struct sPAPRCPUUnplugList
> *unplug_list)
> +{
> +sPAPRCPUUnplug *unplug = g_malloc(sizeof(*unplug));
> +
> +unplug->cpu = cpu;
> +QLIST_INSERT_HEAD(unplug_list, unplug, node);
> +}
> +
> +static int spapr_cpu_release(Object *obj, void *opaque)
> +{
> +DeviceState *dev = DEVICE(obj);
> +CPUState *cs = CPU(dev);
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +struct sPAPRCPUUnplugList *unplug_list = opaque;
> +
> +spapr_cpu_destroy(cpu);
> +cpu_remove_sync(cs);
> +
> +/*
> + * We are still walking the core object's children list, and
> + * hence can't cleanup this CPU thread object just yet. Put
> + * it on a list for later removal.
> + */
> +spapr_add_cpu_to_unplug_list(obj, unplug_list);
> +return 0;
> +}
> +
> +static void spapr_core_release(DeviceState *dev)
> +{
> +struct sPAPRCPUUnplugList unplug_list;
> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> +CPUCore *cc = CPU_CORE(dev);
> +int smt = kvmppc_smt_threads();
> +
> +QLIST_INIT(&unplug_list);
> +object_child_foreach(OBJECT(dev), spapr_cpu_release, &unplug_list);
> +spapr_cpu_core_cleanup(&unplug_list);
> +spapr->cores[cc->core / smt] = NULL;
> +
> +g_free(core->threads);
> +}
> +
> +static void spapr_core_release_unparent(DeviceState *dev, void *opaque)
> +{
> +spapr_core_release(dev);
> +object_unparent(OBJECT(dev));
> +}
> +
> +void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
> + Error **errp)
> +{
> +sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> +PowerPCCPU *cpu = &core->threads[0];
> +int id = ppc_get_vcpu_dt_id(cpu);
> +sPAPRDRConnector *drc =
> +spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id);
> +sPAPRDRConnectorClass *drck;
> +Error *local_err = NULL;
> +
> +g_assert(drc);
> +
> +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +drck->detach(drc, dev, spapr_core_release_unparent, NULL, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> +spapr_hotplug_req_remove_by_index(drc);
> +}
Re: [Qemu-devel] [RFC PATCH v2.1 10/12] spapr: CPU hotplug support
On Thu, Mar 31, 2016 at 02:09:19PM +0530, Bharata B Rao wrote:
> Set up device tree entries for the hotplugged CPU core and use the
> exising RTAS event logging infrastructure to send CPU hotplug notification
> to the guest.
>
> Signed-off-by: Bharata B Rao
Reviewed-by: David Gibson
> ---
> hw/ppc/spapr.c | 58 ++
> hw/ppc/spapr_cpu_core.c | 70
> +
> hw/ppc/spapr_events.c | 3 ++
> hw/ppc/spapr_rtas.c | 24 ++
> include/hw/ppc/spapr.h | 2 ++
> include/hw/ppc/spapr_cpu_core.h | 2 ++
> 6 files changed, 159 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1ead043..1a5dbd9 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -603,6 +603,16 @@ static void spapr_populate_cpu_dt(CPUState *cs, void
> *fdt, int offset,
> size_t page_sizes_prop_size;
> uint32_t vcpus_per_socket = smp_threads * smp_cores;
> uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
> +sPAPRDRConnector *drc;
> +sPAPRDRConnectorClass *drck;
> +int drc_index;
> +
> +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index);
> +if (drc) {
> +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +drc_index = drck->get_index(drc);
> +_FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index)));
> +}
It seems like now we have a concrete representation for cpu cores, we
ought to be passing a cpu core instead of a thread state to the dt
populate function. I guess that complicates backwards compat stuff
for older machine types though.
> /* Note: we keep CI large pages off for now because a 64K capable guest
> * provisioned with large pages might otherwise try to map a qemu
> @@ -987,6 +997,16 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
> _FDT(spapr_drc_populate_dt(fdt, 0, NULL,
> SPAPR_DR_CONNECTOR_TYPE_LMB));
> }
>
> +if (smc->dr_cpu_enabled) {
> +int offset = fdt_path_offset(fdt, "/cpus");
> +ret = spapr_drc_populate_dt(fdt, offset, NULL,
> +SPAPR_DR_CONNECTOR_TYPE_CPU);
> +if (ret < 0) {
> +error_report("Couldn't set up CPU DR device tree properties");
> +exit(1);
> +}
> +}
> +
> _FDT((fdt_pack(fdt)));
>
> if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
> @@ -1622,6 +1642,8 @@ static void spapr_boot_set(void *opaque, const char
> *boot_device,
> void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> {
> CPUPPCState *env = &cpu->env;
> +CPUState *cs = CPU(cpu);
> +int i;
>
> /* Set time-base frequency to 512 MHz */
> cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> @@ -1646,6 +1668,14 @@ void spapr_cpu_init(sPAPRMachineState *spapr,
> PowerPCCPU *cpu, Error **errp)
> }
> }
>
> +/* Set NUMA node for the added CPUs */
> +for (i = 0; i < nb_numa_nodes; i++) {
> +if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) {
> +cs->numa_node = i;
> +break;
> +}
> +}
> +
> xics_cpu_setup(spapr->icp, cpu);
>
> qemu_register_reset(spapr_cpu_reset, cpu);
> @@ -1825,6 +1855,11 @@ static void ppc_spapr_init(MachineState *machine)
>
> for (i = 0; i < spapr_max_cores; i++) {
> int core_dt_id = i * smt;
> +sPAPRDRConnector *drc =
> +spapr_dr_connector_new(OBJECT(spapr),
> + SPAPR_DR_CONNECTOR_TYPE_CPU,
> core_dt_id);
> +
> +qemu_register_reset(spapr_drc_reset, drc);
>
> if (i < spapr_cores) {
> char *type = spapr_get_cpu_core_type(machine->cpu_model);
> @@ -2247,6 +2282,27 @@ out:
> error_propagate(errp, local_err);
> }
>
> +void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
> +sPAPRMachineState *spapr)
> +{
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +DeviceClass *dc = DEVICE_GET_CLASS(cs);
> +int id = ppc_get_vcpu_dt_id(cpu);
> +void *fdt;
> +int offset, fdt_size;
> +char *nodename;
> +
> +fdt = create_device_tree(&fdt_size);
> +nodename = g_strdup_printf("%s@%x", dc->fw_name, id);
> +offset = fdt_add_subnode(fdt, 0, nodename);
> +
> +spapr_populate_cpu_dt(cs, fdt, offset, spapr);
> +g_free(nodename);
> +
> +*fdt_offset = offset;
> +return fdt;
> +}
> +
Ugh. I've really got to find time to clean up a bunch of the DT
construction stuff in spapr. Not something in the scope of this
patch, though.
> static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
> {
> @@ -2287,6 +2343,8 @@ static void spapr_machine_device_plug(HotplugHandler
> *hotplug_dev,
> }
>
> spapr_memory_plug(hotplug_dev, dev, no
[Qemu-devel] [PATCH] vl: Move cpu_synchronize_all_states() into qemu_system_reset()
There are currently 3 calls to qemu_system_reset() in vl.c. Two of them
are immediately preceded by a cpu_synchronize_all_states9) and the
remaining one should be.
The one which doesn't is the very first reset called directly from main().
Without a cpu_synchronize_all_states(), kvm_vcpu_dirty is false at this
point from the earlier cpu_synchronize_all_post_init(). That's incorrect
because the reset path is quite likely to update the CPU state, and that
updated state should be pushed back to KVM, not overwritten with stale
data pushed to KVM immediately after init.
This patch moves the call to cpu_synchronize_all_states() into
qemu_system_reset() for safety, so it is always called. AFAICT this should
be safe for the handful of callers outside vl.c - these all appear to be in
places where the cpu state is already synchronized so the extra call
will be a no-op.
Signed-off-by: David Gibson
---
vl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
This fixes a real but on ppc - the incorrect state of kvm_vcpu_dirty
means that an extra cpu_synchronize_state() in revised code for
updating the MSR is not a no-op as expected and loads a stale MSR
value, resulting in an incorrect MSR value on entry to the guest.
Therefore, I'm hoping to apply this ASAP to 2.6.
Laurent, could you please verify that this does indeed address the
problem with an incorrect MSR on entry.
diff --git a/vl.c b/vl.c
index bd81ea9..3629336 100644
--- a/vl.c
+++ b/vl.c
@@ -1745,6 +1745,8 @@ void qemu_system_reset(bool report)
mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL;
+cpu_synchronize_all_states();
+
if (mc && mc->reset) {
mc->reset();
} else {
@@ -1893,7 +1895,6 @@ static bool main_loop_should_exit(void)
}
if (qemu_reset_requested()) {
pause_all_vcpus();
-cpu_synchronize_all_states();
qemu_system_reset(VMRESET_REPORT);
resume_all_vcpus();
if (!runstate_check(RUN_STATE_RUNNING) &&
@@ -1903,7 +1904,6 @@ static bool main_loop_should_exit(void)
}
if (qemu_wakeup_requested()) {
pause_all_vcpus();
-cpu_synchronize_all_states();
qemu_system_reset(VMRESET_SILENT);
notifier_list_notify(&wakeup_notifiers, &wakeup_reason);
wakeup_reason = QEMU_WAKEUP_REASON_NONE;
--
2.5.5
Re: [Qemu-devel] [PATCH for-2.6] spapr_drc: enable immediate detach for unsignalled devices
On Thu, Mar 31, 2016 at 10:47:24PM -0500, Michael Roth wrote:
> Quoting David Gibson (2016-03-31 21:33:25)
> > On Thu, Mar 31, 2016 at 05:27:37PM -0500, Michael Roth wrote:
> > > Currently spapr doesn't support "aborting" hotplug of PCI
> > > devices by allowing device_del to immediately remove the
> > > device if we haven't signalled the presence of the device
> > > to the guest.
> > >
> > > In the past this wasn't an issue, since we always immediately
> > > signalled device attach and simply relied on full guest-aware
> > > add->remove path for device removal. However, as of 788d259,
> > > we now defer signalling for PCI functions until function 0
> > > is attached, so now we need to deal with these "abort" operations
> > > for cases where a user hotplugs a non-0 function, then opts to
> > > remove it prior hotplugging function 0. Currently they'd have to
> > > reboot before the unplug completed. PCIe multifunction hotplug
> > > does not have this requirement however, so from a management
> > > implementation perspective it would be good to address this within
> > > the same release as 788d259.
> > >
> > > We accomplish this by simply adding a 'signalled' flag to track
> > > whether a device hotplug event has been sent to the guest. If it
> > > hasn't, we allow immediate removal under the assumption that the
> > > guest will not be using the device. Devices present at boot/reset
> > > time are also assumed to be 'signalled'.
> > >
> > > For CPU/memory/etc, signalling will still happen immediately
> > > as part of device_add, so only PCI functions should be affected.
> > >
> > > Cc: bhar...@linux.vnet.ibm.com
> > > Cc: da...@gibson.dropbear.id.au
> > > Cc: sb...@linux.vnet.ibm.com
> > > Cc: qemu-...@nongnu.org
> > > Signed-off-by: Michael Roth
> >
> > So, I'm disinclined to take this during the hard freeze, without some
> > evidence that it fixes a problem case that's really being hit in
> > practice.
>
> The basic situation is that previously we had:
>
> device_add virtio-net-pci,id=hp5.2,addr=0x5.2
> a1) plug device into slot
> a2) signal guest of attach
> a3) guest adds device
> device_del hp5.2
> d1) signal guest of detach
> d2) wait for guest to clean up device and signal completion
> d3) unplug device from slot
>
> After 788d259 we have:
>
> device_add virtio-net-pci,id=hp5.2,addr=0x5.2
> a1) plug device into slot
> a2) defer signalling until we see 0x5.0 added
> device_del hp5.2
> d1) defer signalling removal until we see 0x5.0 deleted
> d2) wait for guest to clean up device and signal completion
>
> But d2) never happens because the guest was never signalled that
> the device was added in the first place.
>
> A real world situation where this might occur is admittedly a bit
> contrived, but in practice I can certainly see it happening:
>
> a) user hotplugs multifunction virtio-net-pci to slot 5, starting
>with function 3 at 0x5.3
> b) user notices they made a mistake, for instance, they forget enable
>vhost in the accompanying netdev
> c) user attempts to unplug function and "abort" the operation, but
>device does not go away
>
> Shiva (on cc:) is working on the libvirt implementation for this
> and hit this in testing. Since it differs in behavior from the PCIe
> workflow it was based on (where aborts are explicitly allowed), and
> causes a minor regression from 2.5, I thought it might be worth
> considering for 2.6, but I can certainly understand if you opt to
> hold off till 2.7.
Ah.. yes, I hadn't registered the fact that this was a regression.
Given that, I think it's probably reasonable for 2.6.
>
> >
> > > ---
> > > hw/ppc/spapr_drc.c | 34 ++
> > > hw/ppc/spapr_events.c | 11 +++
> > > include/hw/ppc/spapr_drc.h | 2 ++
> > > 3 files changed, 47 insertions(+)
> > >
> > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > > index ef063c0..5568e44 100644
> > > --- a/hw/ppc/spapr_drc.c
> > > +++ b/hw/ppc/spapr_drc.c
> > > @@ -173,6 +173,12 @@ static void set_configured(sPAPRDRConnector *drc)
> > > drc->configured = true;
> > > }
> > >
> > > +/* has the guest been notified of device attachment? */
> > > +static void set_signalled(sPAPRDRConnector *drc)
> > > +{
> > > +drc->signalled = true;
> > > +}
> > > +
> > > /*
> > > * dr-entity-sense sensor value
> > > * returned via get-sensor-state RTAS calls
> > > @@ -355,6 +361,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState
> > > *d, void *fdt,
> > > drc->fdt = fdt;
> > > drc->fdt_start_offset = fdt_start_offset;
> > > drc->configured = coldplug;
> > > +drc->signalled = coldplug;
> > >
> > > object_property_add_link(OBJECT(drc), "device",
> > > object_get_typename(OBJECT(drc->dev)),
> > > @@ -371,6 +378,26 @@ static void detach(sPAPRDRConnector *drc,
> > > DeviceState *d,
> > > drc->detach_cb = detach_cb;
> > > drc->detach_cb_opaque = detach_
[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le
Result of doing qemu-system-ppc64 -m 1024 -vnc :1 -net nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive file=my-seed.img,if=virtio ** Attachment added: "crash.png" https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1563887/+attachment/4622485/+files/crash.png -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1563887 Title: qemu-system-ppc64 freezes on starting image on ppc64le Status in QEMU: New Status in linux package in Ubuntu: Confirmed Status in qemu package in Ubuntu: Confirmed Bug description: qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an image as part of the certification process. This on an IBM ppc64le in PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There is no error output. ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive file=seed.iso,if=virtio WARNING: Image format was not specified for 'seed.iso' and probing guessed raw. Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted. Specify the 'raw' format explicitly to remove the restrictions. SLOF ** QEMU Starting Build Date = Jan 29 2016 18:58:37 FW Version = buildd@ release 20151103 Press "s" to enter Open Firmware. Populating /vdevice methods Populating /vdevice/vty@7100 Populating /vdevice/nvram@7101 Populating /vdevice/l-lan@7102 Populating /vdevice/v-scsi@7103 SCSI: Looking for devices 8200 CD-ROM : "QEMU QEMU CD-ROM 2.5+" Populating /pci@8002000 00 1800 (D) : 1af4 1001virtio [ block ] 00 1000 (D) : 1af4 1001virtio [ block ] 00 0800 (D) : 106b 003fserial bus [ usb-ohci ] 00 (D) : 1234 qemu vga No NVRAM common partition, re-initializing... Installing QEMU fb Scanning USB OHCI: initializing USB Keyboard USB mouse No console specified using screen & keyboard Welcome to Open Firmware Copyright (c) 2004, 2011 IBM Corporation All rights reserved. This program and the accompanying materials are made available under the terms of the BSD License available at http://www.opensource.org/licenses/bsd-license.php Trying to load: from: /pci@8002000/scsi@3 ... E3404: Not a bootable device! Trying to load: from: /pci@8002000/scsi@2 ... Successfully loaded Linux ppc64le #31-Ubuntu SMP F ProblemType: Bug DistroRelease: Ubuntu 16.04 Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6 ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6 Uname: Linux 4.4.0-16-generic ppc64le ApportVersion: 2.20-0ubuntu3 Architecture: ppc64el Date: Wed Mar 30 14:10:01 2016 KvmCmdLine: COMMAND STAT EUID RUID PID PPID %CPU COMMAND kvm-irqfd-clean S< 0 0 1172 2 0.0 [kvm-irqfd-clean] qemu-nbdSsl 0 0 13467 1 0.0 qemu-nbd -c /dev/nbd0 xenial-server-cloudimg-ppc64el-disk1.img qemu-system-ppc Sl+ 1000 1000 18973 18896 101 qemu-system-ppc64 -m 256 -display none -nographic -net nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive file=seed.iso,if=virtio Lsusb: Error: command ['lsusb'] failed with exit code 1: ProcEnviron: TERM=xterm PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro ProcLoadAvg: 1.08 0.94 0.58 2/616 19571 ProcLocks: 1: POSIX ADVISORY WRITE 886 00:13:381 0 EOF 2: POSIX ADVISORY WRITE 1339 00:13:528 0 EOF 3: FLOCK ADVISORY WRITE 1284 00:13:522 0 EOF 4: POSIX ADVISORY WRITE 2281 00:13:563 0 EOF 5: POSIX ADVISORY WRITE 1331 00:13:536 0 EOF ProcSwaps: Filename TypeSizeUsedPriority /swap.img file 8388544 0 -1 ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu Mar 24 22:31:14 UTC 2016 SourcePackage: qemu UpgradeStatus: No upgrade log present (probably fresh install) bootlist: /pci@8002011/pci1014,034A@0/sas/disk@4068402c40 /pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,
Re: [Qemu-devel] [PATCH] xen: write information about supported backends
On 01/04/16 16:56, Stefano Stabellini wrote: > On Wed, 30 Mar 2016, Juergen Gross wrote: >> Add a Xenstore directory for each supported pv backend. This will allow >> Xen tools to decide which backend type to use in case there are >> multiple possibilities. >> >> The information is added under >> /local/domain//device-model//backends >> before the "running" state is written to Xenstore. Using a directory >> for each directory enables us to add parameters for specific backends >> in the future. >> >> In order to reuse the Xenstore directory creation already present in >> hw/xen/xen_devconfig.c move the related functions to >> hw/xen/xen_backend.c where they fit better. > > An interface change like this one, needs a new file under docs (in the > Xen repository) to document it. Please add a reference to it, in the > commit message here. Is the Xenstore path documentation enough? Or do you want something like docs/misc/qemu-deprivilege.txt to be added? > In addition please make sure that this can work with QEMU running as > non-root. What are the permissions of the new xenstore directory? Should work, as I'm using the same functions as any backend in qemu is using. The new directory is read-only for the qemu target domain (same as the backend directories). Juergen
[Qemu-devel] [PATCH] opencores_eth: indicate autonegotiation completion
Indicate that autonegotiation is complete in the MII BMSR. This fixes
networking on xtfpga platform in linux v4.5.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Filippov
---
hw/net/opencores_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c
index 09e239a..c6094fb 100644
--- a/hw/net/opencores_eth.c
+++ b/hw/net/opencores_eth.c
@@ -85,7 +85,7 @@ static void mii_reset(Mii *s)
{
memset(s->regs, 0, sizeof(s->regs));
s->regs[MII_BMCR] = 0x1000;
-s->regs[MII_BMSR] = 0x7848; /* no ext regs */
+s->regs[MII_BMSR] = 0x7868; /* no ext regs */
s->regs[MII_PHYIDR1] = 0x2000;
s->regs[MII_PHYIDR2] = 0x5c90;
s->regs[MII_ANAR] = 0x01e1;
--
2.1.4
Re: [Qemu-devel] [PATCH] target-ppc: Correct KVM synchronization for ppc_hash64_set_external_hpt()
On Mon, Apr 04, 2016 at 11:10:56AM +1000, David Gibson wrote:
> On Fri, Apr 01, 2016 at 12:28:31PM +0200, Paolo Bonzini wrote:
> >
> >
> > On 01/04/2016 05:52, David Gibson wrote:
> > > This seems like the right minimal fix in the qemu-2.6 timeframe to fix
> > > the actual bug. However, longer term it seems like the correct thing
> > > to do might be to set kvm_vcpu_dirty early in the reset path. Thoughts?
> >
> > Isn't it done already? vl.c does:
> >
> > pause_all_vcpus();
> > cpu_synchronize_all_states();
> > qemu_system_reset(VMRESET_REPORT);
> > resume_all_vcpus();
>
>
> Huh.. now I'm really confused. Given this I would indeed have
> expected kvm_vpcu_dirty to be set in the reset path, but that would
> make the cpu_synchronize_state() causing the problem to act as a
> no-op, so it shouldn't be causing the problem.
>
> Investigating...
Ok, I've found it.
So, if you reset the system after it's up and running, the fragment
above is indeed what will happen and I think everything will work
correct from there.
The problem occurs only on the very first reset in main() (vl.c:4624
as of de1d099): kvm_vcpu_dirty is false during this call.
Specifically, although kvm_vcpu_dirty was initialized to true in
kvm_vcpu_init(), it gets set to false in cpu_synchronize_post_init()
and never set true again before the qemu_system_reset().
Seems to me we either need a cpu_synchronize_all_states() before that
qemu_system_reset(), or we need to fold the
cpu_synchronize_all_states() right into qemu_system_reset() itself.
Opinion on which option is preferred?
>
> >
> > Thanks,
> >
> > Paolo
> >
> > > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> > > index 72c4ab5..caf41ce 100644
> > > --- a/target-ppc/mmu-hash64.c
> > > +++ b/target-ppc/mmu-hash64.c
> > > @@ -283,8 +283,6 @@ void ppc_hash64_set_external_hpt(PowerPCCPU *cpu,
> > > void *hpt, int shift,
> > > CPUPPCState *env = &cpu->env;
> > > Error *local_err = NULL;
> > >
> > > -cpu_synchronize_state(CPU(cpu));
> > > -
> > > if (hpt) {
> > > env->external_htab = hpt;
> > > } else {
> >
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH 1/2] acpi: tpm: Fix TPM ACPI description (BZ 1281413)
This patch addresses BZ 1281413.
https://bugzilla.redhat.com/show_bug.cgi?id=1281413
Fix the APCI description to make it work on operating systems that are
more strict about the contents of the TPM's ACPI description than Linux
is. The ACPI description was broken in commit 9e472263.
We roll back the ACPI description to where it was in QEMU 2.3.1 and
deactivate the interrupt, modify the scope to \_SB, and change the
name of the device back to 'TPM' from 'ISA.TPM'. Here's the ACPI
description from QEMU 2.3.1:
Scope(\_SB) {
/* TPM with emulated TPM TIS interface */
Device (TPM) {
Name (_HID, EisaID ("PNP0C31"))
Name (_CRS, ResourceTemplate ()
{
Memory32Fixed (ReadWrite, TPM_TIS_ADDR_BASE, TPM_TIS_ADDR_SIZE)
// older Linux tpm_tis drivers do not work with IRQ
//IRQNoFlags () {TPM_TIS_IRQ}
})
Method (_STA, 0, NotSerialized) {
Return (0x0F)
}
}
}
Signed-off-by: Stefan Berger
---
hw/i386/acpi-build.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 35180ef..e11c721 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2335,22 +2335,20 @@ build_dsdt(GArray *table_data, GArray *linker,
Aml *scope = aml_scope("PCI0");
/* Scan all PCI buses. Generate tables to support hotplug. */
build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
-
-if (misc->tpm_version != TPM_VERSION_UNSPEC) {
-dev = aml_device("ISA.TPM");
-aml_append(dev, aml_name_decl("_HID",
aml_eisaid("PNP0C31")));
-aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
-crs = aml_resource_template();
-aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
- TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
-aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
-aml_append(dev, aml_name_decl("_CRS", crs));
-aml_append(scope, dev);
-}
-
-aml_append(sb_scope, scope);
}
}
+
+if (misc->tpm_version != TPM_VERSION_UNSPEC) {
+dev = aml_device("TPM");
+aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
+aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
+crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
+ TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
+//aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
+aml_append(dev, aml_name_decl("_CRS", crs));
+aml_append(sb_scope, dev);
+}
aml_append(dsdt, sb_scope);
}
--
2.5.5
[Qemu-devel] [PATCH 0/2] acpi: tpm: Fix TPM ACPI description
This series of patches fixes some problems with the TPM's ACPI description. Stefan Berger (2): acpi: tpm: Fix TPM ACPI description (BZ 1281413) acpi: tpm: Get the interrupt the device model is using hw/i386/acpi-build.c | 29 ++--- hw/tpm/tpm_tis.c | 5 - include/sysemu/tpm.h | 6 +++--- 3 files changed, 21 insertions(+), 19 deletions(-) -- 2.5.5
[Qemu-devel] [PATCH 2/2] acpi: tpm: Get the interrupt the device model is using
Get the interrupt the TPM device model is using. Do not activate
the interrupt in the ACPI description yet since the current
default value clashes with other devices.
Signed-off-by: Stefan Berger
---
hw/i386/acpi-build.c | 5 +++--
hw/tpm/tpm_tis.c | 5 -
include/sysemu/tpm.h | 6 +++---
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e11c721..ae872a1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -105,6 +105,7 @@ typedef struct AcpiMiscInfo {
bool is_piix4;
bool has_hpet;
TPMVersion tpm_version;
+uint32_t tpm_irq_num;
const unsigned char *dsdt_code;
unsigned dsdt_size;
uint16_t pvpanic_port;
@@ -203,7 +204,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info)
}
info->has_hpet = hpet_find();
-info->tpm_version = tpm_get_version();
+info->tpm_version = tpm_get_parameters(&info->tpm_irq_num);
info->pvpanic_port = pvpanic_port();
info->applesmc_io_base = applesmc_port();
}
@@ -2345,7 +2346,7 @@ build_dsdt(GArray *table_data, GArray *linker,
crs = aml_resource_template();
aml_append(crs, aml_memory32_fixed(TPM_TIS_ADDR_BASE,
TPM_TIS_ADDR_SIZE, AML_READ_WRITE));
-//aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
+//aml_append(crs, aml_irq_no_flags(misc->tpm_irq_num));
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(sb_scope, dev);
}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 381e726..9f5b2f7 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -967,9 +967,12 @@ static int tpm_tis_do_startup_tpm(TPMState *s)
/*
* Get the TPMVersion of the backend device being used
*/
-TPMVersion tpm_tis_get_tpm_version(Object *obj)
+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num)
{
TPMState *s = TPM(obj);
+TPMTISEmuState *tis = &s->s.tis;
+
+*irq_num = tis->irq_num;
return tpm_backend_get_tpm_version(s->be_driver);
}
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index c8afa17..5be45d9 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -26,17 +26,17 @@ typedef enum TPMVersion {
TPM_VERSION_2_0 = 2,
} TPMVersion;
-TPMVersion tpm_tis_get_tpm_version(Object *obj);
+TPMVersion tpm_tis_get_tpm_parameters(Object *obj, uint32_t *irq_num);
#define TYPE_TPM_TIS"tpm-tis"
-static inline TPMVersion tpm_get_version(void)
+static inline TPMVersion tpm_get_parameters(uint32_t *irq_num)
{
#ifdef CONFIG_TPM
Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);
if (obj) {
-return tpm_tis_get_tpm_version(obj);
+return tpm_tis_get_tpm_parameters(obj, irq_num);
}
#endif
return TPM_VERSION_UNSPEC;
--
2.5.5
Re: [Qemu-devel] [PATCH] target-ppc: Correct KVM synchronization for ppc_hash64_set_external_hpt()
On Fri, Apr 01, 2016 at 12:28:31PM +0200, Paolo Bonzini wrote:
>
>
> On 01/04/2016 05:52, David Gibson wrote:
> > This seems like the right minimal fix in the qemu-2.6 timeframe to fix
> > the actual bug. However, longer term it seems like the correct thing
> > to do might be to set kvm_vcpu_dirty early in the reset path. Thoughts?
>
> Isn't it done already? vl.c does:
>
> pause_all_vcpus();
> cpu_synchronize_all_states();
> qemu_system_reset(VMRESET_REPORT);
> resume_all_vcpus();
Huh.. now I'm really confused. Given this I would indeed have
expected kvm_vpcu_dirty to be set in the reset path, but that would
make the cpu_synchronize_state() causing the problem to act as a
no-op, so it shouldn't be causing the problem.
Investigating...
>
> Thanks,
>
> Paolo
>
> > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
> > index 72c4ab5..caf41ce 100644
> > --- a/target-ppc/mmu-hash64.c
> > +++ b/target-ppc/mmu-hash64.c
> > @@ -283,8 +283,6 @@ void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void
> > *hpt, int shift,
> > CPUPPCState *env = &cpu->env;
> > Error *local_err = NULL;
> >
> > -cpu_synchronize_state(CPU(cpu));
> > -
> > if (hpt) {
> > env->external_htab = hpt;
> > } else {
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific core devices
On Fri, Apr 01, 2016 at 11:42:23AM +0530, Bharata B Rao wrote:
> On Fri, Apr 01, 2016 at 04:08:44PM +1100, David Gibson wrote:
> > On Thu, Mar 31, 2016 at 02:09:17PM +0530, Bharata B Rao wrote:
> > > Introduce core devices for each CPU type supported by sPAPR. These
> > > core devices are derived from the base spapr-cpu-core device type.
> > >
> > > TODO:
> > > - Add core types for other remaining CPU types
> > > - Handle CPU model alias correctly
> > >
> > > Signed-off-by: Bharata B Rao
> > > ---
> > > hw/ppc/spapr.c | 3 +-
> > > hw/ppc/spapr_cpu_core.c | 118
> > >
> > > include/hw/ppc/spapr.h | 1 +
> > > include/hw/ppc/spapr_cpu_core.h | 36
> > > 4 files changed, 156 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 64c4acc..45ac5dc 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -1614,8 +1614,7 @@ static void spapr_boot_set(void *opaque, const char
> > > *boot_device,
> > > machine->boot_order = g_strdup(boot_device);
> > > }
> > >
> > > -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> > > - Error **errp)
> > > +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error
> > > **errp)
> > > {
> > > CPUPPCState *env = &cpu->env;
> > >
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index 8cbe2a5..3751a54 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -22,9 +22,127 @@ static const TypeInfo spapr_cpu_core_type_info = {
> > > .instance_size = sizeof(sPAPRCPUCore),
> > > };
> > >
> > > +#define DEFINE_SPAPR_CPU_CORE(_name)
> > > \
> > > +static void
> > > \
> > > +glue(_name, _spapr_cpu_core_create_threads)(DeviceState *dev, int
> > > threads, \
> > > +Error **errp)
> > > \
> > > +{
> > > \
> > > +int i;
> > > \
> > > +Error *local_err = NULL;
> > > \
> > > +sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> > > \
> > > +glue(_name, sPAPRCPUCore) * core =
> > > \
> > > + glue(_name, _SPAPR_CPU_CORE)(OBJECT(dev));
> > > \
> > > +
> > > \
> > > +for (i = 0; i < threads; i++) {
> > > \
> > > +char id[32];
> > > \
> > > +
> > > \
> > > +object_initialize(&sc->threads[i], sizeof(sc->threads[i]),
> > > \
> > > + object_class_get_name(core->cpu));
> > > \
> > > +snprintf(id, sizeof(id), "thread[%d]", i);
> > > \
> > > +object_property_add_child(OBJECT(core), id,
> > > OBJECT(&sc->threads[i]), \
> > > + &local_err);
> > > \
> > > +if (local_err) {
> > > \
> > > +goto err;
> > > \
> > > +}
> > > \
> > > +}
> > > \
> > > +return;
> > > \
> > > +
> > > \
> > > +err:
> > > \
> > > +while (--i) {
> > > \
> > > +object_unparent(OBJECT(&sc->threads[i]));
> > > \
> > > +}
> > > \
> > > +error_propagate(errp, local_err);
> > > \
> > > +}
> > > \
> > > +
> > > \
> > > +static int
> > > \
> > > +glue(_name, _spapr_cpu_core_realize_child)(Object *child, void *opaque)
> > > \
> > > +{
Re: [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific core devices
On Thu, Mar 31, 2016 at 02:09:17PM +0530, Bharata B Rao wrote:
> Introduce core devices for each CPU type supported by sPAPR. These
> core devices are derived from the base spapr-cpu-core device type.
>
> TODO:
> - Add core types for other remaining CPU types
> - Handle CPU model alias correctly
>
> Signed-off-by: Bharata B Rao
> ---
> hw/ppc/spapr.c | 3 +-
> hw/ppc/spapr_cpu_core.c | 118
>
> include/hw/ppc/spapr.h | 1 +
> include/hw/ppc/spapr_cpu_core.h | 36
> 4 files changed, 156 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 64c4acc..45ac5dc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1614,8 +1614,7 @@ static void spapr_boot_set(void *opaque, const char
> *boot_device,
> machine->boot_order = g_strdup(boot_device);
> }
>
> -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> - Error **errp)
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> {
> CPUPPCState *env = &cpu->env;
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 8cbe2a5..3751a54 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -22,9 +22,127 @@ static const TypeInfo spapr_cpu_core_type_info = {
> .instance_size = sizeof(sPAPRCPUCore),
> };
>
> +#define DEFINE_SPAPR_CPU_CORE(_name)
> \
> +static void
> \
> +glue(_name, _spapr_cpu_core_create_threads)(DeviceState *dev, int threads,
> \
> +Error **errp)
> \
> +{
> \
> +int i;
> \
> +Error *local_err = NULL;
> \
> +sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> \
> +glue(_name, sPAPRCPUCore) * core =
> \
> + glue(_name, _SPAPR_CPU_CORE)(OBJECT(dev));
> \
> +
> \
> +for (i = 0; i < threads; i++) {
> \
> +char id[32];
> \
> +
> \
> +object_initialize(&sc->threads[i], sizeof(sc->threads[i]),
> \
> + object_class_get_name(core->cpu));
> \
> +snprintf(id, sizeof(id), "thread[%d]", i);
> \
> +object_property_add_child(OBJECT(core), id, OBJECT(&sc->threads[i]),
> \
> + &local_err);
> \
> +if (local_err) {
> \
> +goto err;
> \
> +}
> \
> +}
> \
> +return;
> \
> +
> \
> +err:
> \
> +while (--i) {
> \
> +object_unparent(OBJECT(&sc->threads[i]));
> \
> +}
> \
> +error_propagate(errp, local_err);
> \
> +}
> \
> +
> \
> +static int
> \
> +glue(_name, _spapr_cpu_core_realize_child)(Object *child, void *opaque)
> \
> +{
> \
> +Error **errp = opaque;
> \
> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> \
> +CPUState *cs = CPU(child);
> \
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> \
> +
> \
> +object_property_set_bool(child, true, "realized", errp);
>
Re: [Qemu-devel] [RFC PATCH v2.1 05/12] qdev: hotplug: Introduce HotplugHandler.pre_plug() callback
On Fri, Apr 01, 2016 at 12:38:28PM +0200, Paolo Bonzini wrote:
>
>
> On 01/04/2016 05:30, David Gibson wrote:
> > On Thu, Mar 31, 2016 at 02:09:14PM +0530, Bharata B Rao wrote:
> >> From: Igor Mammedov
> >>
> >> pre_plug callback is to be called before device.realize() is executed.
> >> This would allow to check/set device's properties from HotplugHandler.
> >>
> >> Signed-off-by: Igor Mammedov
> >> Signed-off-by: Bharata B Rao
> >
> > Reviewed-by: David Gibson
> >
> > It would be really nice to get some opinion on this from Andreas or
> > Paolo.
>
> Certainly okay for me, Igor did all of the HotplugHandler design and
> work.
Ok, good to hear. Thanks.
>
> Paolo
>
> >> ---
> >> hw/core/hotplug.c| 11 +++
> >> hw/core/qdev.c | 9 -
> >> include/hw/hotplug.h | 14 +-
> >> 3 files changed, 32 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> >> index 645cfca..17ac986 100644
> >> --- a/hw/core/hotplug.c
> >> +++ b/hw/core/hotplug.c
> >> @@ -13,6 +13,17 @@
> >> #include "hw/hotplug.h"
> >> #include "qemu/module.h"
> >>
> >> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> >> + DeviceState *plugged_dev,
> >> + Error **errp)
> >> +{
> >> +HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
> >> +
> >> +if (hdc->pre_plug) {
> >> +hdc->pre_plug(plug_handler, plugged_dev, errp);
> >> +}
> >> +}
> >> +
> >> void hotplug_handler_plug(HotplugHandler *plug_handler,
> >>DeviceState *plugged_dev,
> >>Error **errp)
> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >> index db41aa1..a0b3aad 100644
> >> --- a/hw/core/qdev.c
> >> +++ b/hw/core/qdev.c
> >> @@ -1062,6 +1062,14 @@ static void device_set_realized(Object *obj, bool
> >> value, Error **errp)
> >> g_free(name);
> >> }
> >>
> >> +hotplug_ctrl = qdev_get_hotplug_handler(dev);
> >> +if (hotplug_ctrl) {
> >> +hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
> >> +if (local_err != NULL) {
> >> +goto fail;
> >> +}
> >> +}
> >> +
> >> if (dc->realize) {
> >> dc->realize(dev, &local_err);
> >> }
> >> @@ -1072,7 +1080,6 @@ static void device_set_realized(Object *obj, bool
> >> value, Error **errp)
> >>
> >> DEVICE_LISTENER_CALL(realize, Forward, dev);
> >>
> >> -hotplug_ctrl = qdev_get_hotplug_handler(dev);
> >> if (hotplug_ctrl) {
> >> hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
> >> }
> >> diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
> >> index 2db025d..50d84e9 100644
> >> --- a/include/hw/hotplug.h
> >> +++ b/include/hw/hotplug.h
> >> @@ -46,7 +46,8 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
> >> * hardware (un)plug functions.
> >> *
> >> * @parent: Opaque parent interface.
> >> - * @plug: plug callback.
> >> + * @pre_plug: pre plug callback called at start of device.realize(true)
> >> + * @plug: plug callback called at end of device.realize(true).
> >> * @unplug_request: unplug request callback.
> >> * Used as a means to initiate device unplug for devices
> >> that
> >> * require asynchronous unplug handling.
> >> @@ -59,6 +60,7 @@ typedef struct HotplugHandlerClass {
> >> InterfaceClass parent;
> >>
> >> /* */
> >> +hotplug_fn pre_plug;
> >> hotplug_fn plug;
> >> hotplug_fn unplug_request;
> >> hotplug_fn unplug;
> >> @@ -74,6 +76,16 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
> >>Error **errp);
> >>
> >> /**
> >> + * hotplug_handler_pre_plug:
> >> + *
> >> + * Call #HotplugHandlerClass.pre_plug callback of @plug_handler.
> >> + */
> >> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> >> + DeviceState *plugged_dev,
> >> + Error **errp);
> >> +
> >> +
> >> +/**
> >> * hotplug_handler_unplug_request:
> >> *
> >> * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.
> >
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
On 3 April 2016 at 22:42, Liviu Ionescu wrote: >> On 04 Apr 2016, at 00:30, Peter Maydell wrote: >> Which I completely agree with. If you want to send a patch >> to add the assert I'm happy to review it. > > I guess the original report was clear enough for an active commiter > to easily understand the problem and fix it, in significantly less > iterations that I would need to produce a compliant and acceptable > patch (reaching this point already took 9 messages). :-( There are many many bits of suboptimal code, things that could be improved and outright bugs in QEMU. Fixing any of them takes time, which is why as an open source project we encourage people to contribute the fixes they personally run into and find they need. (This is pretty much what anybody who is an active committer is already doing.) thanks -- PMM
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
> On 04 Apr 2016, at 00:30, Peter Maydell wrote: > > On 3 April 2016 at 18:56, Liviu Ionescu wrote: >> well, to summarise, I notified you that in certain conditions, >> due to an non-obvious dependency that does not break the build >> when not met, the existing code crashes at run time with a >> segmentation fault, and I suggested that an assert might help >> developers to save some debugging time. > > Which I completely agree with. If you want to send a patch > to add the assert I'm happy to review it. I guess the original report was clear enough for an active commiter to easily understand the problem and fix it, in significantly less iterations that I would need to produce a compliant and acceptable patch (reaching this point already took 9 messages). :-( regards, Liviu
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
On 3 April 2016 at 18:56, Liviu Ionescu wrote: > well, to summarise, I notified you that in certain conditions, > due to an non-obvious dependency that does not break the build > when not met, the existing code crashes at run time with a > segmentation fault, and I suggested that an assert might help > developers to save some debugging time. Which I completely agree with. If you want to send a patch to add the assert I'm happy to review it. thanks -- PMM
Re: [Qemu-devel] [PATCH] virtio-blk: assert on starting/stopping
On 03/04/2016 21:59, Christian Borntraeger wrote: > Thread 1 (Thread 0x3ffad25bb90 (LWP 41685)): > ---Type to continue, or q to quit--- > #0 0x03ffab5be2c0 in raise () at /lib64/libc.so.6 > #1 0x03ffab5bfc26 in abort () at /lib64/libc.so.6 > #2 0x03ffab5b5bce in __assert_fail_base () at /lib64/libc.so.6 > #3 0x03ffab5b5c5c in () at /lib64/libc.so.6 > #4 0x800b79e4 in virtio_blk_data_plane_start (s=0x80b195a0) at > /home/cborntra/REPOS/qemu/hw/block/dataplane/virtio-blk.c:210 > #5 0x800b57ba in virtio_blk_handle_output (vdev=0x80e4b0f8, > vq=0x80eaa180) at /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:607 > #6 0x800f0d74 in virtio_queue_notify_vq (vq=0x80eaa180) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1108 > #7 0x800f376c in virtio_queue_host_notifier_read (n=0x80eaa1e0) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1820 > #8 0x800f381c in virtio_queue_set_host_notifier_fd_handler > (vq=0x80eaa180, assign=false, set_handler=false) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1836 > #9 0x8010b808 in virtio_ccw_set_guest2host_notifier (dev=0x80e49fb0, > n=0, assign=false, set_handler=false) at > /home/cborntra/REPOS/qemu/hw/s390x/virtio-ccw.c:98 > #10 0x8010baaa in virtio_ccw_stop_ioeventfd (dev=0x80e49fb0) at > /home/cborntra/REPOS/qemu/hw/s390x/virtio-ccw.c:155 > #11 0x8010f162 in virtio_ccw_set_host_notifier (d=0x80e49fb0, n=0, > assign=true) at /home/cborntra/REPOS/qemu/hw/s390x/virtio-ccw.c:1212 > #12 0x800b7ab0 in virtio_blk_data_plane_start (s=0x80b195a0) at > /home/cborntra/REPOS/qemu/hw/block/dataplane/virtio-blk.c:225 > #13 0x800b57ba in virtio_blk_handle_output (vdev=0x80e4b0f8, > vq=0x80eaa180) at /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:607 > #14 0x800f0d74 in virtio_queue_notify_vq (vq=0x80eaa180) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1108 > #15 0x800f376c in virtio_queue_host_notifier_read (n=0x80eaa1e0) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1820 > #16 0x802f1a0c in aio_dispatch (ctx=0x80abae30) at > /home/cborntra/REPOS/qemu/aio-posix.c:327 > #17 0x802df4d4 in aio_ctx_dispatch (source=0x80abae30, callback=0x0, > user_data=0x0) at /home/cborntra/REPOS/qemu/async.c:233 > #18 0x03ffabfd1c0a in g_main_context_dispatch () at > /lib64/libglib-2.0.so.0 > #19 0x802ee70e in glib_pollfds_poll () at > /home/cborntra/REPOS/qemu/main-loop.c:213 > #20 0x802ee84a in os_host_main_loop_wait (timeout=147200) at > /home/cborntra/REPOS/qemu/main-loop.c:258 > #21 0x802ee956 in main_loop_wait (nonblocking=0) at > /home/cborntra/REPOS/qemu/main-loop.c:506 > #22 0x8017dc0c in main_loop () at /home/cborntra/REPOS/qemu/vl.c:1934 > #23 0x801860e0 in main (argc=72, argv=0x3ffd60fe838, > envp=0x3ffd60fea80) at /home/cborntra/REPOS/qemu/vl.c:4652 This will be fixed by Cornelia's rework, and is an example of why I think patch 1/9 is a good idea (IOW, assign=false is harmful). Thanks, Paolo
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
it shows: -rwxr-xr-x 1 root root 265848 يول 18 2014 /sbin/init 2016-04-03 21:13 GMT+01:00 Marwa Hamza : > it shows: > -rwxr-xr-x 1 root root 265848 يول 18 2014 /sbin/init > > > 2016-04-03 20:49 GMT+01:00 Marwa Hamza : > >> the output of this command > ./i386-softmmu/qemu-system-i386 -M pc >> -kernel >> > >> /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage >> > -initrd >> /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz >> > -append “root=/dev/ram rdinit=/sbin/init” >> > >> starting init :/sbin/init exists but couldn't execute it (error -8) >> starting init : /bin/sh exists but couldn't execute it (error -8) >> kernel panic not syncing : no working init found , try passing init= >> option to kernel >> >> 2016-04-03 19:38 GMT+01:00 Pranith Kumar : >> >>> On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza >>> wrote: >>> >>> > ./i386-softmmu/qemu-system-i386 -M pc -kernel >>> > >>> /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage >>> > -initrd >>> /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz >>> > -append “root=/dev/ram rdinit=/sbin/init” >>> > >>> >>> Can you post the output when you run this command? In particular, does >>> the /sbin/init exist in the rootfs? >>> >>> -- >>> Pranith >>> >> >> >
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
it shows: -rwxr-xr-x 1 root root 265848 يول 18 2014 /sbin/init 2016-04-03 20:49 GMT+01:00 Marwa Hamza : > the output of this command > ./i386-softmmu/qemu-system-i386 -M pc > -kernel > > > /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage > > -initrd > /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz > > -append “root=/dev/ram rdinit=/sbin/init” > > > starting init :/sbin/init exists but couldn't execute it (error -8) > starting init : /bin/sh exists but couldn't execute it (error -8) > kernel panic not syncing : no working init found , try passing init= > option to kernel > > 2016-04-03 19:38 GMT+01:00 Pranith Kumar : > >> On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza >> wrote: >> >> > ./i386-softmmu/qemu-system-i386 -M pc -kernel >> > >> /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage >> > -initrd >> /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz >> > -append “root=/dev/ram rdinit=/sbin/init” >> > >> >> Can you post the output when you run this command? In particular, does >> the /sbin/init exist in the rootfs? >> >> -- >> Pranith >> > >
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
On Sun, Apr 3, 2016 at 3:49 PM, Marwa Hamza wrote: > the output of this command > ./i386-softmmu/qemu-system-i386 -M pc -kernel >> >> /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage >> -initrd >> /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz >> -append “root=/dev/ram rdinit=/sbin/init” >> > starting init :/sbin/init exists but couldn't execute it (error -8) > starting init : /bin/sh exists but couldn't execute it (error -8) > kernel panic not syncing : no working init found , try passing init= option > to kernel > I don't think this is a qemu problem. From the error message it looks like init in your busybox root image is not configured properly. I would focus on finding why the init file is not able to run(permissions, maybe?). -- Pranith
Re: [Qemu-devel] [PATCH] virtio-blk: assert on starting/stopping
On 04/03/2016 12:37 PM, Michael S. Tsirkin wrote: > Reentrancy cannot happen while the BQL is being held, > so we should never enter this condition. > > Cc: Christian Borntraeger > Cc: Cornelia Huck > Cc: Paolo Bonzini > Signed-off-by: Michael S. Tsirkin > --- > > This is a replacement for [PATCH 9/9] virtio: remove starting/stopping > checks Christian, could you please give it a spin with debug enabled? > Since you reported above Paolo's patch triggers segfaults, I expect this > one to trigger assertions as well, which should give us more info on > the root cause. > the assert triggered (see below). (gdb) thread apply all bt Thread 5 (Thread 0x3ffa9fff910 (LWP 41714)): #0 0x03ffab68841e in syscall () at /lib64/libc.so.6 #1 0x803e84f6 in futex_wait (ev=0x80a65bd4 , val=4294967295) at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:292 #2 0x803e8786 in qemu_event_wait (ev=0x80a65bd4 ) at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:399 #3 0x80405ec4 in call_rcu_thread (opaque=0x0) at /home/cborntra/REPOS/qemu/util/rcu.c:250 #4 0x03ffab787c2c in start_thread () at /lib64/libpthread.so.0 #5 0x03ffab68ec9a in thread_start () at /lib64/libc.so.6 Thread 4 (Thread 0x3ffa97ff910 (LWP 41718)): #0 0x8001b09a in address_space_read_continue (as=0x805da230 , addr=350645744, attrs=..., buf=0x3ffa97f8450 "", len=0, addr1=350645728, l=16, mr=0x80b0d6a0) at /home/cborntra/REPOS/qemu/exec.c:2738 #1 0x8001b186 in address_space_read_full (as=0x805da230 , addr=350645728, attrs=..., buf=0x3ffa97f8440 "\230\001q\024", len=16) at /home/cborntra/REPOS/qemu/exec.c:2752 #2 0x800ed284 in vring_desc_read (len=16, buf=0x3ffa97f8440 "\230\001q\024", attrs=..., addr=350645728, as=0x805da230 ) at /home/cborntra/REPOS/qemu/include/exec/memory.h:1431 #3 0x800ed284 in vring_desc_read (vdev=0x80e44b88, desc=0x3ffa97f8440, desc_pa=350645696, i=2) at /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:124 #4 0x800ee05e in virtqueue_read_next_desc (vdev=0x80e44b88, desc=0x3ffa97f8440, desc_pa=350645696, max=3) at /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:366 #5 0x800eecbe in virtqueue_pop (vq=0x80f221c0, sz=160) at /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:602 #6 0x800b40b0 in virtio_blk_get_request (s=0x80e44b88) at /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:192 #7 0x800b56e0 in virtio_blk_handle_vq (s=0x80e44b88, vq=0x80f221c0) at /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:588 #8 0x800b78a2 in virtio_blk_data_plane_handle_output (vdev=0x80e44b88, vq=0x80f221c0) at /home/cborntra/REPOS/qemu/hw/block/dataplane/virtio-blk.c:195 #9 0x800f0cb4 in virtio_queue_notify_aio_vq (vq=0x80f221c0) at /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1098 #10 0x800f3664 in virtio_queue_host_notifier_aio_read (n=0x80f0) at /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1799 #11 0x802f1a0c in aio_dispatch (ctx=0x80acd5d0) at /home/cborntra/REPOS/qemu/aio-posix.c:327 #12 0x802f2392 in aio_poll (ctx=0x80acd5d0, blocking=true) at /home/cborntra/REPOS/qemu/aio-posix.c:475 #13 0x8016590a in iothread_run (opaque=0x80acd090) at /home/cborntra/REPOS/qemu/iothread.c:46 #14 0x03ffab787c2c in start_thread () at /lib64/libpthread.so.0 #15 0x03ffab68ec9a in thread_start () at /lib64/libc.so.6 Thread 3 (Thread 0x3ff8b9d2910 (LWP 41739)): #0 0x03ffab68334a in ioctl () at /lib64/libc.so.6 #1 0x80081c84 in kvm_vcpu_ioctl (cpu=0x80e4d2b0, type=44672) at /home/cborntra/REPOS/qemu/kvm-all.c:1984 #2 0x8008154c in kvm_cpu_exec (cpu=0x80e4d2b0) at /home/cborntra/REPOS/qemu/kvm-all.c:1834 #3 0x8006075c in qemu_kvm_cpu_thread_fn (arg=0x80e4d2b0) at /home/cborntra/REPOS/qemu/cpus.c:1056 #4 0x03ffab787c2c in start_thread () at /lib64/libpthread.so.0 #5 0x03ffab68ec9a in thread_start () at /lib64/libc.so.6 Thread 2 (Thread 0x3ff8b1d2910 (LWP 41743)): #0 0x03ffab68334a in ioctl () at /lib64/libc.so.6 #1 0x80081c84 in kvm_vcpu_ioctl (cpu=0x80b40040, type=44672) at /home/cborntra/REPOS/qemu/kvm-all.c:1984 #2 0x8008154c in kvm_cpu_exec (cpu=0x80b40040) at /home/cborntra/REPOS/qemu/kvm-all.c:1834 #3 0x8006075c in qemu_kvm_cpu_thread_fn (arg=0x80b40040) at /home/cborntra/REPOS/qemu/cpus.c:1056 #4 0x03ffab787c2c in start_thread () at /lib64/libpthread.so.0 #5 0x03ffab68ec9a in thread_start () at /lib64/libc.so.6 Thread 1 (Thread 0x3ffad25bb90 (LWP 41685)): ---Type to continue, or q to quit--- #0 0x03ffab5be2c0 in raise () at /lib64/libc.so.6 #1 0x03ffab5bfc26 in abort () at /lib64/libc.so.6 #2 0x03ffab5b5bce in __assert_fail_base () at /lib64/libc.so.6 #3 0x03ffab5b5c5c in () at /lib64/libc.so.6 #4 0x800b79e4 in virtio_blk_data_plane_start (s=0x80b195a0) at /home/cborntra/REPOS/qemu/hw/block/dataplane/virtio-blk.c:210 #5 0x800b57ba
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
the output of this command > ./i386-softmmu/qemu-system-i386 -M pc -kernel > /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage > -initrd /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz > -append “root=/dev/ram rdinit=/sbin/init” > starting init :/sbin/init exists but couldn't execute it (error -8) starting init : /bin/sh exists but couldn't execute it (error -8) kernel panic not syncing : no working init found , try passing init= option to kernel 2016-04-03 19:38 GMT+01:00 Pranith Kumar : > On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza > wrote: > > > ./i386-softmmu/qemu-system-i386 -M pc -kernel > > > /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage > > -initrd > /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz > > -append “root=/dev/ram rdinit=/sbin/init” > > > > Can you post the output when you run this command? In particular, does > the /sbin/init exist in the rootfs? > > -- > Pranith >
[Qemu-devel] [Bug 1565395] Re: qemu-2.4.1 fails when compiled against pulseaudio
** Changed in: qemu
Status: New => Confirmed
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1565395
Title:
qemu-2.4.1 fails when compiled against pulseaudio
Status in QEMU:
Confirmed
Bug description:
If I compile qemu-2.4.1 like this:
CC="gcc -mtune=generic -Os -pipe" CXX="g++ -mtune=generic -Os -pipe
-fno-exceptions -fno-rtti" ./configure --prefix=/usr/local
--localstatedir=/var --libexecdir=/usr/local/lib/qemu
--interp-prefix=/usr/local/share/qemu --disable-smartcard-nss
--disable-curses --disable-brlapi --audio-drv-list="oss alsa sdl"
--target-list="i386-softmmu i386-linux-user x86_64-softmmu
x86_64-linux-user" --smbd=/usr/local/sbin/smbd
find . -name config-host.mak -type f -exec sed -i 's/-O2//g' {} \;
make
..it works fine.
If I add pulseaudio dev files and use --audio-drv-list="oss alsa sdl pa",
then "qemu-system-x86_64 -blah-blah" opens a window, displays the bios
message and stops. Strace shows qemu is not hung, but loops continually.
The same happens with qemu-2.2.1 and qemu-2.5.0.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1565395/+subscriptions
[Qemu-devel] [ RFC Patch v4 3/3] virtio-net rsc: support coalescing ipv6 tcp traffic
From: Wei Xu
Most things like ipv4 except there is a significant difference between ipv4
and ipv6, the fragment lenght in ipv4 header includes itself, while it's not
included for ipv6, thus means ipv6 can carry a real '65535' payload.
Signed-off-by: Wei Xu
---
hw/net/virtio-net.c | 147 +---
1 file changed, 141 insertions(+), 6 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 81e8e71..2d09352 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -50,6 +50,10 @@
/* header lenght value in ip header without option */
#define VIRTIO_NET_IP4_HEADER_LENGTH 5
+#define ETH_IP6_HDR_SZ (ETH_HDR_SZ + IP6_HDR_SZ)
+#define VIRTIO_NET_IP6_ADDR_SIZE 32 /* ipv6 saddr + daddr */
+#define VIRTIO_NET_MAX_IP6_PAYLOAD VIRTIO_NET_MAX_TCP_PAYLOAD
+
/* Purge coalesced packets timer interval */
#define VIRTIO_NET_RSC_INTERVAL 30
@@ -1725,6 +1729,25 @@ static void virtio_net_rsc_extract_unit4(NetRscChain
*chain,
unit->payload = htons(*unit->ip_plen) - ip_hdrlen - unit->tcp_hdrlen;
}
+static void virtio_net_rsc_extract_unit6(NetRscChain *chain,
+ const uint8_t *buf, NetRscUnit* unit)
+{
+uint16_t hdr_len;
+struct ip6_header *ip6;
+
+hdr_len = ((VirtIONet *)(chain->n))->guest_hdr_len;
+ip6 = (struct ip6_header *)(buf + hdr_len + sizeof(struct eth_header));
+unit->ip = ip6;
+unit->ip_plen = &(ip6->ip6_ctlun.ip6_un1.ip6_un1_plen);
+unit->tcp = (struct tcp_header *)(((uint8_t *)unit->ip)\
++ sizeof(struct ip6_header));
+unit->tcp_hdrlen = (htons(unit->tcp->th_offset_flags) & 0xF000) >> 10;
+
+/* There is a difference between payload lenght in ipv4 and v6,
+ ip header is excluded in ipv6 */
+unit->payload = htons(*unit->ip_plen) - unit->tcp_hdrlen;
+}
+
static void virtio_net_rsc_ipv4_checksum(struct ip_header *ip)
{
uint32_t sum;
@@ -1738,7 +1761,9 @@ static size_t virtio_net_rsc_drain_seg(NetRscChain
*chain, NetRscSeg *seg)
{
int ret;
-virtio_net_rsc_ipv4_checksum(seg->unit.ip);
+if ((chain->proto == ETH_P_IP) && seg->is_coalesced) {
+virtio_net_rsc_ipv4_checksum(seg->unit.ip);
+}
ret = virtio_net_do_receive(seg->nc, seg->buf, seg->size);
QTAILQ_REMOVE(&chain->buffers, seg, next);
g_free(seg->buf);
@@ -1804,7 +1829,18 @@ static void virtio_net_rsc_cache_buf(NetRscChain *chain,
NetClientState *nc,
QTAILQ_INSERT_TAIL(&chain->buffers, seg, next);
chain->stat.cache++;
-virtio_net_rsc_extract_unit4(chain, seg->buf, &seg->unit);
+switch (chain->proto) {
+case ETH_P_IP:
+virtio_net_rsc_extract_unit4(chain, seg->buf, &seg->unit);
+break;
+
+case ETH_P_IPV6:
+virtio_net_rsc_extract_unit6(chain, seg->buf, &seg->unit);
+break;
+
+default:
+g_assert_not_reached();
+}
}
static int32_t virtio_net_rsc_handle_ack(NetRscChain *chain, NetRscSeg *seg,
@@ -1948,6 +1984,24 @@ static int32_t virtio_net_rsc_coalesce4(NetRscChain
*chain, NetRscSeg *seg,
return virtio_net_rsc_coalesce_data(chain, seg, buf, unit);
}
+static int32_t virtio_net_rsc_coalesce6(NetRscChain *chain, NetRscSeg *seg,
+const uint8_t *buf, size_t size, NetRscUnit *unit)
+{
+struct ip6_header *ip1, *ip2;
+
+ip1 = (struct ip6_header *)(unit->ip);
+ip2 = (struct ip6_header *)(seg->unit.ip);
+if (memcmp(&ip1->ip6_src, &ip2->ip6_src, sizeof(struct in6_address))
+|| memcmp(&ip1->ip6_dst, &ip2->ip6_dst, sizeof(struct in6_address))
+|| (unit->tcp->th_sport ^ seg->unit.tcp->th_sport)
+|| (unit->tcp->th_dport ^ seg->unit.tcp->th_dport)) {
+chain->stat.no_match++;
+return RSC_NO_MATCH;
+}
+
+return virtio_net_rsc_coalesce_data(chain, seg, buf, unit);
+}
+
/* Pakcets with 'SYN' should bypass, other flag should be sent after drain
* to prevent out of order */
static int virtio_net_rsc_tcp_ctrl_check(NetRscChain *chain,
@@ -1991,7 +2045,11 @@ static size_t virtio_net_rsc_do_coalesce(NetRscChain
*chain, NetClientState *nc,
NetRscSeg *seg, *nseg;
QTAILQ_FOREACH_SAFE(seg, &chain->buffers, next, nseg) {
-ret = virtio_net_rsc_coalesce4(chain, seg, buf, size, unit);
+if (chain->proto == ETH_P_IP) {
+ret = virtio_net_rsc_coalesce4(chain, seg, buf, size, unit);
+} else {
+ret = virtio_net_rsc_coalesce6(chain, seg, buf, size, unit);
+}
if (ret == RSC_FINAL) {
if (virtio_net_rsc_drain_seg(chain, seg) == 0) {
@@ -2116,13 +2174,82 @@ static size_t virtio_net_rsc_receive4(void *opq,
NetClientState* nc,
return virtio_net_rsc_do_coalesce(chain, nc, buf, size, &unit);
}
+static int32_t virtio_net_rsc_sanity_check6(NetRscChain *chain,
+struct ip6_header *ip6,
+
[Qemu-devel] [ RFC Patch v4 2/3] virtio-net rsc: support coalescing ipv4 tcp traffic
From: Wei Xu
All the data packets in a tcp connection will be cached to a big buffer
in every receive interval, and will be sent out via a timer, the
'virtio_net_rsc_timeout' controls the interval, the value will influent the
performance and response of tcp connection extremely, 5(50us) is a
experience value to gain a performance improvement, since the whql test
sends packets every 100us, so '30(300us)' can pass the test case,
this is also the default value, it's gonna to be tunable.
The timer will only be triggered if the packets pool is not empty,
and it'll drain off all the cached packets
'NetRscChain' is used to save the segments of different protocols in a
VirtIONet device.
The main handler of TCP includes TCP window update, duplicated ACK check
and the real data coalescing if the new segment passed sanity check
and is identified as an 'wanted' one.
An 'wanted' segment means:
1. Segment is within current window and the sequence is the expected one.
2. ACK of the segment is in the valid window.
3. If the ACK in the segment is a duplicated one, then it must less than 2,
this is to notify upper layer TCP starting retransmission due to the spec.
Sanity check includes:
1. Incorrect version in IP header
2. IP options & IP fragment
3. Not a TCP packets
4. Sanity size check to prevent buffer overflow attack.
There maybe more cases should be considered such as ip identification other
flags, while it broke the test because windows set it to the same even it's
not a fragment.
Normally it includes 2 typical ways to handle a TCP control flag, 'bypass'
and 'finalize', 'bypass' means should be sent out directly, and 'finalize'
means the packets should also be bypassed, and this should be done
after searching for the same connection packets in the pool and sending
all of them out, this is to avoid out of data.
All the 'SYN' packets will be bypassed since this always begin a new'
connection, other flags such 'FIN/RST' will trigger a finalization, because
this normally happens upon a connection is going to be closed, an 'URG' packet
also finalize current coalescing unit.
Statistics can be used to monitor the basic coalescing status, the 'out of
order'
and 'out of window' means how many retransmitting packets, thus describe the
performance intuitively.
Signed-off-by: Wei Xu
---
hw/net/virtio-net.c| 480 -
include/hw/virtio/virtio-net.h | 1 +
include/hw/virtio/virtio.h | 72 +++
3 files changed, 552 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index bd91a4b..81e8e71 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -15,10 +15,12 @@
#include "qemu/iov.h"
#include "hw/virtio/virtio.h"
#include "net/net.h"
+#include "net/eth.h"
#include "net/checksum.h"
#include "net/tap.h"
#include "qemu/error-report.h"
#include "qemu/timer.h"
+#include "qemu/sockets.h"
#include "hw/virtio/virtio-net.h"
#include "net/vhost_net.h"
#include "hw/virtio/virtio-bus.h"
@@ -38,6 +40,24 @@
#define endof(container, field) \
(offsetof(container, field) + sizeof(((container *)0)->field))
+#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */
+#define VIRTIO_NET_TCP_PORT_SIZE 4/* sport + dport */
+
+/* IPv4 max payload, 16 bits in the header */
+#define VIRTIO_NET_MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header))
+#define VIRTIO_NET_MAX_TCP_PAYLOAD 65535
+
+/* header lenght value in ip header without option */
+#define VIRTIO_NET_IP4_HEADER_LENGTH 5
+
+/* Purge coalesced packets timer interval */
+#define VIRTIO_NET_RSC_INTERVAL 30
+
+/* This value affects the performance a lot, and should be tuned carefully,
+ '30'(300us) is the recommended value to pass the WHQL test, '5' can
+ gain 2x netperf throughput with tso/gso/gro 'off'. */
+static uint32_t virtio_net_rsc_timeout = VIRTIO_NET_RSC_INTERVAL;
+
typedef struct VirtIOFeature {
uint32_t flags;
size_t end;
@@ -1688,11 +1708,467 @@ static int virtio_net_load_device(VirtIODevice *vdev,
QEMUFile *f,
return 0;
}
+static void virtio_net_rsc_extract_unit4(NetRscChain *chain,
+ const uint8_t *buf, NetRscUnit* unit)
+{
+uint16_t hdr_len;
+uint16_t ip_hdrlen;
+struct ip_header *ip;
+
+hdr_len = ((VirtIONet *)(chain->n))->guest_hdr_len;
+ip = (struct ip_header *)(buf + hdr_len + sizeof(struct eth_header));
+unit->ip = (void *)ip;
+ip_hdrlen = (ip->ip_ver_len & 0xF) << 2;
+unit->ip_plen = &ip->ip_len;
+unit->tcp = (struct tcp_header *)(((uint8_t *)unit->ip) + ip_hdrlen);
+unit->tcp_hdrlen = (htons(unit->tcp->th_offset_flags) & 0xF000) >> 10;
+unit->payload = htons(*unit->ip_plen) - ip_hdrlen - unit->tcp_hdrlen;
+}
+
+static void virtio_net_rsc_ipv4_checksum(struct ip_header *ip)
+{
+uint32_t sum;
+
+ip->ip_sum = 0;
+sum = net_checksum_add_cont(sizeof(struct ip_header), (uint8_t *)ip, 0);
+ip->
[Qemu-devel] [ RFC Patch v4 1/3] virtio-net rsc: add a new host offload(rsc) feature bit
From: Wei Xu
A new feature bit 'VIRTIO_NET_F_GUEST_RSC' is introduced to support WHQL
Receive-Segment-Offload test, this feature will coalesce tcp packets in
IPv4/6 for userspace virtio-net driver.
This feature can be enabled either by 'ACK' the feature when loading
the driver in the guest, or by sending the 'VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET'
command to the host via control queue.
Signed-off-by: Wei Xu
---
hw/net/virtio-net.c | 29 +++--
include/standard-headers/linux/virtio_net.h | 1 +
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 5798f87..bd91a4b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -537,6 +537,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev,
uint64_t features,
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
+virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_RSC);
}
if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) {
@@ -582,7 +583,8 @@ static uint64_t
virtio_net_guest_offloads_by_features(uint32_t features)
(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
(1ULL << VIRTIO_NET_F_GUEST_TSO6) |
(1ULL << VIRTIO_NET_F_GUEST_ECN) |
-(1ULL << VIRTIO_NET_F_GUEST_UFO);
+(1ULL << VIRTIO_NET_F_GUEST_UFO) |
+(1ULL << VIRTIO_NET_F_GUEST_RSC);
return guest_offloads_mask & features;
}
@@ -1089,7 +1091,8 @@ static int receive_filter(VirtIONet *n, const uint8_t
*buf, int size)
return 0;
}
-static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
+static ssize_t virtio_net_do_receive(NetClientState *nc,
+ const uint8_t *buf, size_t size)
{
VirtIONet *n = qemu_get_nic_opaque(nc);
VirtIONetQueue *q = virtio_net_get_subqueue(nc);
@@ -1685,6 +1688,26 @@ static int virtio_net_load_device(VirtIODevice *vdev,
QEMUFile *f,
return 0;
}
+
+static ssize_t virtio_net_rsc_receive(NetClientState *nc,
+ const uint8_t *buf, size_t size)
+{
+return virtio_net_do_receive(nc, buf, size);
+}
+
+static ssize_t virtio_net_receive(NetClientState *nc,
+ const uint8_t *buf, size_t size)
+{
+VirtIONet *n;
+
+n = qemu_get_nic_opaque(nc);
+if (n->curr_guest_offloads & VIRTIO_NET_F_GUEST_RSC) {
+return virtio_net_rsc_receive(nc, buf, size);
+} else {
+return virtio_net_do_receive(nc, buf, size);
+}
+}
+
static NetClientInfo net_virtio_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
@@ -1909,6 +1932,8 @@ static Property virtio_net_properties[] = {
TX_TIMER_INTERVAL),
DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
+DEFINE_PROP_BIT("guest_rsc", VirtIONet, host_features,
+VIRTIO_NET_F_GUEST_RSC, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/standard-headers/linux/virtio_net.h
b/include/standard-headers/linux/virtio_net.h
index a78f33e..5b95762 100644
--- a/include/standard-headers/linux/virtio_net.h
+++ b/include/standard-headers/linux/virtio_net.h
@@ -55,6 +55,7 @@
#define VIRTIO_NET_F_MQ22 /* Device supports Receive Flow
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_GUEST_RSC 24 /* Guest can coalesce tcp packets */
#ifndef VIRTIO_NET_NO_LEGACY
#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
--
2.5.0
[Qemu-devel] [ RFC Patch v4 0/3] Support Receive-Segment-Offload(RSC) for WHQL
From: Wei Xu Changes in V4: - Add new host feature bit - Replace using fixed header lenght with dynamic header lenght in VirtIONet - Change ip/ip6 header union in NetRscUnit to void* pointer - Add macro prefix, adjust code indent, etc. Changes in V3: - Removed big param list, replace it with 'NetRscUnit' - Different virtio header size - Modify callback function to direct call. - Needn't check the failure of g_malloc() - Other code format adjustment, macro naming, etc Changes in V2: - Add detailed commit log This patch is to support WHQL test for Windows guest, while this feature also benifits other guest works as a kernel 'gro' like feature with userspace implementation. Feature information: http://msdn.microsoft.com/en-us/library/windows/hardware/jj853324 Both IPv4 and IPv6 are supported, though performance with userspace virtio is slow than vhost-net, there is about 1.5x to 2x performance improvement to userspace virtio, this is done by turning this feature on and disable 'tso/gso/gro' on corresponding tap interface and guest interface, while get less improment with all these feature on. Linux guest performance data(Netperf): MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.2.101 () port 0 AF_INET : nodelay Size SizeSize Time Throughput bytes bytes bytessecs.10^6bits/sec 87380 16384 646.00 1221.20 87380 16384 646.00 1260.30 87380 163841286.00 1978.51 87380 163841286.00 2286.05 87380 163842566.00 2677.94 87380 163842566.00 4615.42 87380 163845126.00 2956.54 87380 163845126.00 5356.39 87380 16384 10246.00 2798.17 87380 16384 10246.00 4943.30 87380 16384 20486.00 2681.09 87380 16384 20486.00 4835.81 87380 16384 40966.00 3390.14 87380 16384 40966.00 5391.54 87380 16384 80926.00 3008.27 87380 16384 80926.00 5381.68 87380 16384 102406.00 2999.89 87380 16384 102406.00 5393.11 Test steps: Although this feature is mainly used for window guest, i used linux guest to help test the feature, to make things simple, i used 3 steps to test the patch as i moved on. 1. With a tcp socket client/server pair running on 2 linux guest, thus i can control the traffic and debugging the code as i want. 2. Netperf on linux guest test the throughput. 3. WHQL test with 2 Windows guests. Current status: IPv4 pass all the above tests. IPv6 just passed test step 1 and 2 as described ahead, the virtio nic cannot receive any packet in WHQL test, trying with a win debug binary Note: A 'MessageDevice' nic chose as 'Realtek' will panic the system sometimes during setup, this can be figured out by replacing it with an 'e1000' nic. Todo: More sanity check and tcp 'ecn' and 'window' scale test. Wei Xu (3): virtio-net rsc: add a new host offload(rsc) feature bit virtio-net rsc: support coalescing ipv4 tcp traffic virtio-net rsc: support coalescing ipv6 tcp traffic hw/net/virtio-net.c | 642 +++- include/hw/virtio/virtio-net.h | 1 + include/hw/virtio/virtio.h | 72 include/standard-headers/linux/virtio_net.h | 1 + 4 files changed, 714 insertions(+), 2 deletions(-) -- 2.5.0
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza wrote: > ./i386-softmmu/qemu-system-i386 -M pc -kernel > /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage > -initrd /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz > -append “root=/dev/ram rdinit=/sbin/init” > Can you post the output when you run this command? In particular, does the /sbin/init exist in the rootfs? -- Pranith
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
but i tried the same thing with arm arch ( file system made by busybox) and i used sh shell and it worked well 2016-04-03 20:20 GMT+02:00 Pranith Kumar : > On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza > wrote: > > hello , i tried to run qemu on x64 system , > > > > those are steps that i followed > > i compile the kernel 4.4.1 with arch =i386 > > i download busybox 1.21.0 > > make ARCH=i386 menuconfig > > I checked the option to compile Busybox as a static executable > > make ARCH=i386 install > > cd _install > > mkdir proc sys dev lib etc etc/init.d > > gedit etc/inittab > > ::sysinit:/etc/init.d/rcS > > sudo chmod +x etc/inittab > > sudo gedit etc/init.d/rcS > > #!/bin/sh > > Can you try changing this line to: "#!/sbin/ash"? > > I am not sure busybox has sh shell installed or configured properly. > That is what your error message is pointing to atleast. > > > > starting init :/bin/sh exists but couldn’t execute it > > kernel panic – not syncing no working init found > > > Thanks! > -- > Pranith >
Re: [Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
On Sun, Apr 3, 2016 at 9:50 AM, Marwa Hamza wrote: > hello , i tried to run qemu on x64 system , > > those are steps that i followed > i compile the kernel 4.4.1 with arch =i386 > i download busybox 1.21.0 > make ARCH=i386 menuconfig > I checked the option to compile Busybox as a static executable > make ARCH=i386 install > cd _install > mkdir proc sys dev lib etc etc/init.d > gedit etc/inittab > ::sysinit:/etc/init.d/rcS > sudo chmod +x etc/inittab > sudo gedit etc/init.d/rcS > #!/bin/sh Can you try changing this line to: "#!/sbin/ash"? I am not sure busybox has sh shell installed or configured properly. That is what your error message is pointing to atleast. > starting init :/bin/sh exists but couldn’t execute it > kernel panic – not syncing no working init found Thanks! -- Pranith
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] spapr: compute interrupt vector address from LPCR
On 04/01/2016 04:43 AM, David Gibson wrote:
> On Thu, Mar 31, 2016 at 08:13:09AM +0100, Mark Cave-Ayland wrote:
>> On 31/03/16 05:55, David Gibson wrote:
>>
>>> On Wed, Mar 30, 2016 at 05:38:34PM +0200, Cédric Le Goater wrote:
This address is changed by the linux kernel using the H_SET_MODE hcall
and needs to be restored when migrating a spapr VM running in
TCG. This can be done using the AIL bits from the LPCR register.
The patch introduces a helper routine cpu_ppc_get_excp_prefix() which
returns the effective address offset of the interrupt handler
depending on the LPCR_AIL bits. The same helper can be used in the
H_SET_MODE hcall, which lets us remove the H_SET_MODE_ADDR_TRANS_*
defines.
Signed-off-by: Cédric Le Goater
>>>
>>> I've applied this (with Greg's minor amendments) to ppc-for-2.6),
>>> since it certainly improves behaviour, although I have a couple of
>>> queries:
>>>
---
Changes since v1:
- moved helper routine under target-ppc/
- moved the restore of excp_prefix under cpu_post_load()
hw/ppc/spapr_hcall.c| 13 ++---
include/hw/ppc/spapr.h |5 -
target-ppc/cpu.h|9 +
target-ppc/machine.c| 20 +++-
target-ppc/translate_init.c | 14 ++
5 files changed, 44 insertions(+), 17 deletions(-)
Index: qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c
===
--- qemu-dgibson-for-2.6.git.orig/hw/ppc/spapr_hcall.c
+++ qemu-dgibson-for-2.6.git/hw/ppc/spapr_hcall.c
@@ -835,17 +835,8 @@ static target_ulong h_set_mode_resource_
return H_P4;
}
-switch (mflags) {
-case H_SET_MODE_ADDR_TRANS_NONE:
-prefix = 0;
-break;
-case H_SET_MODE_ADDR_TRANS_0001_8000:
-prefix = 0x18000;
-break;
-case H_SET_MODE_ADDR_TRANS_C000___4000:
-prefix = 0xC0004000ULL;
-break;
-default:
+prefix = cpu_ppc_get_excp_prefix(mflags);
+if (prefix == (target_ulong) -1ULL) {
return H_UNSUPPORTED_FLAG;
}
Index: qemu-dgibson-for-2.6.git/target-ppc/machine.c
===
--- qemu-dgibson-for-2.6.git.orig/target-ppc/machine.c
+++ qemu-dgibson-for-2.6.git/target-ppc/machine.c
@@ -156,12 +156,26 @@ static void cpu_pre_save(void *opaque)
}
}
+
+static int cpu_post_load_excp_prefix(CPUPPCState *env)
+{
+int ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
+target_ulong prefix = cpu_ppc_get_excp_prefix(ail);
+
+if (prefix == (target_ulong) -1ULL) {
+return -EINVAL;
+}
+env->excp_prefix = prefix;
+return 0;
+}
+
static int cpu_post_load(void *opaque, int version_id)
{
PowerPCCPU *cpu = opaque;
CPUPPCState *env = &cpu->env;
int i;
target_ulong msr;
+int ret = 0;
/*
* We always ignore the source PVR. The user or management
@@ -201,7 +215,11 @@ static int cpu_post_load(void *opaque, i
hreg_compute_mem_idx(env);
-return 0;
+if (env->spr[SPR_LPCR] & LPCR_AIL) {
+ret = cpu_post_load_excp_prefix(env);
+}
>>>
>>> Why not call this unconditionally? If AIL == 0 it will still do the
>>> right thing.
>>>
>>> Aren't there also circumstances where the exception prefix can depend
>>> on the MSR? Do those need to be handled somewhere?
>>
>> Yes indeed - this was part of my patchset last year to fix up various
>> migration issues for the Mac PPC machines (see commit
>> 2360b6e84f78d41fa0f76555a947148b73645259).
>>
>> I agree that having the env->excp_prefix logic split like this isn't a
>> particularly great idea. Let's just have a single helper function as in
>> the patch above and use that in both places (and in fact with recent
>> changes I have a feeling env->excp_prefix is one of the few remaining
>> reasons that the above change is still required, but please check this).
>
> Right.
>
> So, what I'd really prefer to see here is a single
> update_excp_prefix() helper which will set env->excp_prefix based on
> whatever registers are relevant (LPCR, MSR and probably the cpu
> model). That would be called on incoming migration, and after
> updating any of the relevant registers (so from the mtmsr and mtlpcr
> emulations). H_SET_MODE should be handled by first updating the LPCR
> value, then calling the update helper.
>
> Cédric, I'm ok if you provide a version of that which only handles
> POWER7 and POWER8 (i.e. spapr compatible models for now).
Sure.
But first, could you please tak
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
> On 03 Apr 2016, at 20:01, Peter Maydell wrote: > > ... This is you changing QEMU to not compile a file that was > previously always compiled. If you do that then it's > not surprising if things might break when you move to an > upstream version, that's all. well, to summarise, I notified you that in certain conditions, due to an non-obvious dependency that does not break the build when not met, the existing code crashes at run time with a segmentation fault, and I suggested that an assert might help developers to save some debugging time. if you want to add this assert, ok; if you don't, and prefer to permanently blame my fork for various reasons, it's also fine, I don't mind, I fix it in my fork and move forward. regards, Liviu
Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users
On 03/04/2016 15:49, Michael S. Tsirkin wrote: > > I agree but I think we need a better name for this function. > qemu_ram_offset_to_ptr? > Will also serve to make sure backporting patches across this > API change does not cause issues. Yes, this makes sense. If it were all in 2.6, there would be no released version with an absolute ram_addr_t argument *and* a RAMBlock* argument, but we will do the incompatible change in 2.7 and then it makes sense to rename it. Paolo
Re: [Qemu-devel] [PATCH 8/9] virtio: merge virtio_queue_aio_set_host_notifier_handler with virtio_queue_set_aio
On 03/04/2016 11:06, Michael S. Tsirkin wrote:
> On Fri, Apr 01, 2016 at 03:19:53PM +0200, Paolo Bonzini wrote:
>> Eliminating the reentrancy is actually a nice thing that we can do
>> with the API that Michael proposed, so let's make it first class.
>> This also hides the complex assign/set_handler conventions from
>> callers of virtio_queue_aio_set_host_notifier_handler, which in
>> fact was always called with assign=true.
>>
>> Reviewed-by: Cornelia Huck
>> Signed-off-by: Paolo Bonzini
>> ---
>> hw/block/dataplane/virtio-blk.c | 7 +++
>> hw/scsi/virtio-scsi-dataplane.c | 12
>> hw/virtio/virtio.c | 19 ---
>> include/hw/virtio/virtio.h | 6 ++
>> 4 files changed, 13 insertions(+), 31 deletions(-)
>>
>> diff --git a/hw/block/dataplane/virtio-blk.c
>> b/hw/block/dataplane/virtio-blk.c
>> index fd06726..74c6d37 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -237,8 +237,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>>
>> /* Get this show started by hooking up our callbacks */
>> aio_context_acquire(s->ctx);
>> -virtio_set_queue_aio(s->vq, virtio_blk_data_plane_handle_output);
>> -virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true);
>> +virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx,
>> +
>> virtio_blk_data_plane_handle_output);
>> aio_context_release(s->ctx);
>> return;
>>
>> @@ -273,8 +273,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>> aio_context_acquire(s->ctx);
>>
>> /* Stop notifications for new requests from guest */
>> -virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, false);
>> -virtio_set_queue_aio(s->vq, NULL);
>> +virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, NULL);
>>
>> /* Drain and switch bs back to the QEMU main loop */
>> blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
>> diff --git a/hw/scsi/virtio-scsi-dataplane.c
>> b/hw/scsi/virtio-scsi-dataplane.c
>> index a497a2c..5494dcc 100644
>> --- a/hw/scsi/virtio-scsi-dataplane.c
>> +++ b/hw/scsi/virtio-scsi-dataplane.c
>> @@ -81,8 +81,7 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue
>> *vq, int n,
>> return rc;
>> }
>>
>> -virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, true, true);
>> -virtio_set_queue_aio(vq, fn);
>> +virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, fn);
>> return 0;
>> }
>>
>> @@ -99,13 +98,10 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
>> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
>> int i;
>>
>> -virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, true,
>> false);
>> -virtio_set_queue_aio(vs->ctrl_vq, NULL);
>> -virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, true,
>> false);
>> -virtio_set_queue_aio(vs->event_vq, NULL);
>> +virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, NULL);
>> +virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, NULL);
>> for (i = 0; i < vs->conf.num_queues; i++) {
>> -virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
>> true, false);
>> -virtio_set_queue_aio(vs->cmd_vqs[i], NULL);
>> +virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
>> NULL);
>> }
>> }
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index eb04ac0..7fcfc24f 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -1159,14 +1159,6 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
>> queue_size,
>> return &vdev->vq[i];
>> }
>>
>> -void virtio_set_queue_aio(VirtQueue *vq,
>> - void (*handle_output)(VirtIODevice *, VirtQueue
>> *))
>> -{
>> -assert(vq->handle_output);
>> -
>> -vq->handle_aio_output = handle_output;
>> -}
>> -
>> void virtio_del_queue(VirtIODevice *vdev, int n)
>> {
>> if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
>> @@ -1809,19 +1801,16 @@ static void
>> virtio_queue_host_notifier_aio_read(EventNotifier *n)
>> }
>>
>> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext
>> *ctx,
>> -bool assign, bool
>> set_handler)
>> +void
>> (*handle_output)(VirtIODevice *,
>> + VirtQueue *))
>> {
>> -if (assign && set_handler) {
>> +vq->handle_aio_output = handle_output;
>> +if (handle_output) {
>> aio_set_event_notifier(ctx, &vq->host_notifier, true,
>> virtio_queue_host_notifier_aio_read);
>> } else {
>> aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL);
>> }
>> -if (!assign) {
>> -/* Test and clear notifier before after dis
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
On 3 April 2016 at 17:57, Liviu Ionescu wrote: > >> On 03 Apr 2016, at 19:43, Peter Maydell wrote: >> >> On 3 April 2016 at 16:40, Liviu Ionescu wrote: >>> On 03 Apr 2016, at 15:28, Peter Maydell wrote: >>> since hw/arm/boot.o is in obj-y it should always be built, >>> >>> not necessarily, in my build configuration I have if's that >>> exclude most of the files non related to Cortex-M. >> >> boot.o is in obj-y; it is (I think) impossible to >> build a v7M supporting QEMU without boot.o unless you've modified >> the makefiles for some reason. > > that's nothing more than the usual make magic, most of the makefiles look > like this: > > > ifeq ($(CONFIG_GNU_ARM_ECLIPSE),n) This is you changing QEMU to not compile a file that was previously always compiled. If you do that then it's not surprising if things might break when you move to an upstream version, that's all. Ideally we'd have more CONFIG_SOME_DEVICE and CONFIG_SOME_BOARD checks here the way we do for the more recent boards like the Xilinx and iMX ones, then you could disable boards you didn't want in the config file. thanks -- PMM
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
> On 03 Apr 2016, at 19:43, Peter Maydell wrote: > > On 3 April 2016 at 16:40, Liviu Ionescu wrote: >> >>> On 03 Apr 2016, at 15:28, Peter Maydell wrote: >> >>> since hw/arm/boot.o is in obj-y it should always be built, >> >> not necessarily, in my build configuration I have if's that >> exclude most of the files non related to Cortex-M. > > boot.o is in obj-y; it is (I think) impossible to > build a v7M supporting QEMU without boot.o unless you've modified > the makefiles for some reason. that's nothing more than the usual make magic, most of the makefiles look like this: ifeq ($(CONFIG_GNU_ARM_ECLIPSE),n) obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o obj-$(CONFIG_DIGIC) += digic_boards.o obj-y += integratorcp.o mainstone.o musicpal.o nseries.o obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o obj-$(CONFIG_ACPI) += virt-acpi-build.o obj-y += netduino2.o obj-y += sysbus-fdt.o obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o obj-$(CONFIG_DIGIC) += digic.o obj-y += omap1.o omap2.o strongarm.o obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o endif regards, Liviu
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
On 3 April 2016 at 16:40, Liviu Ionescu wrote: > >> On 03 Apr 2016, at 15:28, Peter Maydell wrote: > >> since hw/arm/boot.o is in obj-y it should always be built, > > not necessarily, in my build configuration I have if's that > exclude most of the files non related to Cortex-M. boot.o is in obj-y; it is (I think) impossible to build a v7M supporting QEMU without boot.o unless you've modified the makefiles for some reason. That would not be a configuration issue or a bug in QEMU, but a problem with your local modifications. > in previous versions boot.o was not needed. in 2.5.1 you > introduced a non-obvious dependency to it and the build > passed, but the program crashed. > >> but we could assert on it rather than just crashing. > > my suggestion is to assert, it is easier to debug a failed > assertion. Yes, I agree. thanks -- PMM
[Qemu-devel] run qemu on x64 system ( ARCH=i386 or ARCH=x86-64) and on x86 system
hello , i tried to run qemu on x64 system , those are steps that i followed i compile the kernel 4.4.1 with arch =i386 i download busybox 1.21.0 make ARCH=i386 menuconfig I checked the option to compile Busybox as a static executable make ARCH=i386 install cd _install mkdir proc sys dev lib etc etc/init.d gedit etc/inittab ::sysinit:/etc/init.d/rcS sudo chmod +x etc/inittab sudo gedit etc/init.d/rcS #!/bin/sh echo “hello” // hello display when starting the qemu mount -t proc none /proc mount -t sysfs none /sys /sbin/mdev -s sudo chmod +x _install/etc/init.d/rcS find . | cpio -o –format=newc > ../rootfs.img cd .. gzip -c rootfs.img > rootfs.img.gz ./i386-softmmu/qemu-system-i386 -M pc -kernel /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage -initrd /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz -append “root=/dev/ram rdinit=/sbin/init” i have tried this command too ./i386-softmmu/qemu-system-i386 -M pc -kernel /home/marwa/Bureau/lauterbach/i386_qemu/linux-4.1.18/arch/i386/boot/bzImage -initrd /home/marwa/Bureau/lauterbach/i386_qemu/busybox-1.21.0/rootfs.img.gz -append “root=/dev/ram rdinit=/bin/sh” but i got this msg in qemu ‘shell starting init :/bin/sh exists but couldn’t execute it kernel panic – not syncing no working init found should i work with another file system ? or create a new one , I got the same error with x86 , , any one please can help me to get qemu working perfectly thanks
Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix sending mouse event to guest
On Apr 3, 2016, at 8:21 AM, Peter Maydell wrote:
> On 2 April 2016 at 18:53, Programmingkid wrote:
>>
>> On Apr 2, 2016, at 1:35 PM, Peter Maydell wrote:
>>
>>> On 2 April 2016 at 18:25, Programmingkid wrote:
On Apr 2, 2016, at 1:07 PM, Peter Maydell wrote:
> On 2 April 2016 at 17:56, Programmingkid
> wrote:
>> The mouse down event should not be sent to the guest if the mouse down
>> event
>> causes an activation of QEMU. This patch prevents activation clicks from
>> going
>> to the guest.
>>
>> Signed-off-by: John Arbuckle
>> ---
>> ui/cocoa.m | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 60a7c07..07d9c86 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -698,7 +698,7 @@ QemuCocoaView *cocoaView;
>>* call below. We definitely don't want to pass that click through
>>* to the guest.
>>*/
>> -if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
>> +if ((isMouseGrabbed && [[self window] isKeyWindow]) &&
>> (last_buttons != buttons)) {
>> static uint32_t bmap[INPUT_BUTTON__MAX] = {
>> [INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON,
>> --
>> 2.7.2
>
> I'm afraid I don't really understand why you think this
> should change. On the face of it the current code looks right:
> we pass through the mouse button if:
> (1) we've got the mouse grab
> or (2) our window has the focus, even if it's not grabbed
>
> I would expect the "activation click" to be "we don't have
> the mouse grab, and we don't have focus either (some other
> app is foreground)".
When QEMU's main window is in the background and the user clicks on it,
the NSLeftMouseUp case in the handleEvent: method will set the
isMouseGrabbed variable to true. This means the
"if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
(last_buttons != buttons))" code will always be true for a left
mouse button click. The mouse event will be sent to the guest
when it shouldn't be.
>>>
>>> OK, that sounds like a bug, but this doesn't look like the
>>> right way to fix it, because with your change we won't
>>> pass through the click if this is a click on the window
>>> when it's not in the background.
>>
>> I think logically this is the best solution. If the window is in
>> the foreground and doesn't have the mouse grabbed, the user can't
>> move the mouse pointer in the guest. This could cause stuff to be
>> clicked that might cause undesirable effects.
>
> So why do we now have a different condition for "don't pass
> through buttons" and "don't pass through mouse events" ?
>
> The process for making changes should be something like:
> * identify the model we are trying to use to decide whether
> to pass events through to the guest or not
> * if this model is wrong (ie there is a design flaw),
> identify what the changes to the design need to be
> (possibly looking at other OSX VM UIs and how we handle
> similar cases in our other UI frontends, to identify what
> might be reasonable behaviour), so that we have a coherent
> design which we can use now and in the future to fix the code
> * work out where the code is diverging from the design
> (either because the code is buggy or because we made a
> change to the design in the previous step)
> * change the code
> * make sure that comments match the changed code/design
>
> I don't have strong feelings about what the right design for
> handling mouse events should be, but I do think we should
> have one, and this patch seems to be making a local change
> rather than considering the larger picture.
Maybe the mouse cursor in the guest should be able to move even when the mouse
isn't grabbed. So if the mouse is grabbed or not in QEMU, the mouse pointer in
the guest should always move when the host mouse pointer is in the QEMU window.
If this is true, then sending mouse button events to the guest would be ok
whether grabbed or not because the user can just move the mouse out of the way
of danger at any time. You may have to read this several times before it makes
sense.
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
> On 03 Apr 2016, at 15:28, Peter Maydell wrote: > since hw/arm/boot.o is in obj-y it should always be built, not necessarily, in my build configuration I have if's that exclude most of the files non related to Cortex-M. in previous versions boot.o was not needed. in 2.5.1 you introduced a non-obvious dependency to it and the build passed, but the program crashed. > but we could assert on it rather than just crashing. my suggestion is to assert, it is easier to debug a failed assertion. regards, Liviu
Re: [Qemu-devel] [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for R6 and MSA instructions
Hello, Leon, thank you very much for the kind feedback. Let me clarify my take
on the involved issues.
1) Class operations
I am going to correct the code as you hinted.
The reason I wanted separate handling of MSA class operation is code and module
decoupling. Handling of MSA instructions (in file msa_helper.c) and regular
instructions (in file op_helper.c) have many overlaping areas - however, my
understanding is that the designer of MSA module wanted it to be as independant
on code in other files/modulas as possible. Handling class operation is on of
the rare instances where code in msa_helper.c relies on the code in
op_helper.c., and it made sense to me that this dependence should be removed,
for the sake of consistency within MSA module - even if the functionalitied are
virtually identical. That said, I will anyway listen to your advice, since you
most probably see more than myself regarding this, and I am going to revert to
a single handling of class operations, for both MSA and regular versions.
2) Flush subnormals
My impression is that his set of features should be treated and implemented
separately, at some later point in time.
Although the implementation seems not to be too complex (defining FCR31_FS,
invoking appropriately set_flush_to_zero() and set_flush_inputs_to_zero() on
CPU init, plus special exception handling, like it is already done for MSA
equivalents), it looks to me that it would have added a lot of risk into a
patch series that is already touching a lot of sensitive areas, and therefore
introducing a lot of risks. Once this patch series is hopefully intergrated,
flush subnormals will be much easier to integrate, since it will be mips-only
issue. Therefore, if you agree, I will leave it for the future. I will
definitely mention it in commit messages though (as a limitaion), for future
reference.
Thanks again for your consideration of this matter.
Sincerely yours,
Aleksandar
Original Message
Subject: Re: [PATCH 2/2] target-mips: Implement IEEE 754-2008 functionality for
R6 and MSA instructions
Date: Friday, April 1, 2016 21:07 CEST
From: Leon Alrae
To: Aleksandar Markovic ,
CC: , ,
,, ,
,, ,
,, ,
,,
,,
,,
References:
<1458910214-12239-1-git-send-email-aleksandar.marko...@rt-rk.com><1458910214-12239-3-git-send-email-aleksandar.marko...@rt-rk.com>
On 25/03/16 12:50, Aleksandar Markovic wrote:
> +#define MSA_CLASS_SIGNALING_NAN 0x001
> +#define MSA_CLASS_QUIET_NAN 0x002
> +#define MSA_CLASS_NEGATIVE_INFINITY 0x004
> +#define MSA_CLASS_NEGATIVE_NORMAL 0x008
> +#define MSA_CLASS_NEGATIVE_SUBNORMAL 0x010
> +#define MSA_CLASS_NEGATIVE_ZERO 0x020
> +#define MSA_CLASS_POSITIVE_INFINITY 0x040
> +#define MSA_CLASS_POSITIVE_NORMAL 0x080
> +#define MSA_CLASS_POSITIVE_SUBNORMAL 0x100
> +#define MSA_CLASS_POSITIVE_ZERO 0x200
> +
> +#define MSA_CLASS(name, bits) \
> +uint ## bits ## _t helper_msa_ ## name (CPUMIPSState *env, \
> + uint ## bits ## _t arg) \
> +{ \
> + if (float ## bits ## _is_signaling_nan(arg, \
> + &env->active_tc.msa_fp_status)) { \
> + return MSA_CLASS_SIGNALING_NAN; \
> + } else if (float ## bits ## _is_quiet_nan(arg, \
> + &env->active_tc.msa_fp_status)) { \
> + return MSA_CLASS_QUIET_NAN; \
> + } else if (float ## bits ## _is_neg(arg)) { \
> + if (float ## bits ## _is_infinity(arg)) { \
> + return MSA_CLASS_NEGATIVE_INFINITY; \
> + } else if (float ## bits ## _is_zero(arg)) { \
> + return MSA_CLASS_NEGATIVE_ZERO; \
> + } else if (float ## bits ## _is_zero_or_denormal(arg)) { \
> + return MSA_CLASS_NEGATIVE_SUBNORMAL; \
> + } else { \
> + return MSA_CLASS_NEGATIVE_NORMAL; \
> + } \
> + } else { \
> + if (float ## bits ## _is_infinity(arg)) { \
> + return MSA_CLASS_POSITIVE_INFINITY; \
> + } else if (float ## bits ## _is_zero(arg)) { \
> + return MSA_CLASS_POSITIVE_ZERO; \
> + } else if (float ## bits ## _is_zero_or_denormal(arg)) { \
> + return MSA_CLASS_POSITIVE_SUBNORMAL; \
> + } else { \
> + return MSA_CLASS_POSITIVE_NORMAL; \
> + } \
> + } \
> +}
Duplicating the class operation is unnecessary. We can just have common
function for FPU and MSA which takes additional float_status argument.
Also I noticed that this patch series doesn't provide Flush Subnormals
(the FCSR.FS bit), but probably this functionality can come later...
Leon
Re: [Qemu-devel] [PATCH 1/2] softfloat: Enable run-time-configurable meaning of signaling NaN bit
I truly appreciate your guidance and bringing this matter to my attention.
It just seems to me that, in similar case, 16-bit default NaN value should be
0x7E00. This value is needed for MSA operations. ("MIPS Architecture for
Programmers Volume IV-j: The MIPS32® SIMD Architecture Module", Revision 1.12,
(february 3, 2016), page 52, table 3.7 "Default NaN Encodings")
I plan to include all three corrections in the next version of this patch set.
Please, let me know if you think that I should not.
Yours,
Aleksandar
From: qemu-devel-bounces+aleksandar.markovic=imgtec@nongnu.org
[qemu-devel-bounces+aleksandar.markovic=imgtec@nongnu.org] on behalf of
Leon Alrae
Sent: Friday, April 01, 2016 12:02 PM
To: Aleksandar Markovic; qemu-devel@nongnu.org
Cc: peter.mayd...@linaro.org; ehabk...@redhat.com; pro...@gmail.com;
mark.cave-ayl...@ilande.co.uk; ag...@suse.de; kbast...@mail.uni-paderborn.de;
Petar Jovanovic; blauwir...@gmail.com; jcmvb...@gmail.com; Miodrag Dinic;
qemu-...@nongnu.org; qemu-...@nongnu.org; edgar.igles...@gmail.com;
pbonz...@redhat.com; g...@mprc.pku.edu.cn; afaer...@suse.de;
aurel...@aurel32.net; r...@twiddle.net
Subject: Re: [Qemu-devel] [PATCH 1/2] softfloat: Enable run-time-configurable
meaning of signaling NaN bit
On 25/03/16 12:50, Aleksandar Markovic wrote:
>
> /*
> | The pattern for a default generated single-precision NaN.
>
> **/
> +float32 float32_default_nan(float_status *status) {
> #if defined(TARGET_SPARC)
> -const float32 float32_default_nan = const_float32(0x7FFF);
> +return const_float32(0x7FFF);
> #elif defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_ALPHA) ||
> \
>defined(TARGET_XTENSA) || defined(TARGET_S390X) ||
> defined(TARGET_TRICORE)
> -const float32 float32_default_nan = const_float32(0x7FC0);
> -#elif SNAN_BIT_IS_ONE
> -const float32 float32_default_nan = const_float32(0x7FBF);
> +return const_float32(0x7FC0);
> #else
> -const float32 float32_default_nan = const_float32(0xFFC0);
> +if (status->snan_bit_is_one)
> +return const_float32(0x7FBF);
> +else
> +return const_float32(0xFFC0);
Here for MIPS (when FCR31.NAN2008 is set) we should generate 0x7FC0
for single-precision. Reference:
"MIPS Architecture For Programmers, Volume I-A: Introduction to the
MIPS64 Architecture", Imagination Technologies LTD., Document Number:
MD00083, Revision 6.01, August 20, 2014, Table 6.3 "Value Supplied When
a New Quiet NaN Is Created", p. 84
Also, for double-precision we should generate 0x7FF8.
Thanks,
Leon
Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users
On Thu, Mar 24, 2016 at 12:03:35PM +0100, Paolo Bonzini wrote:
> Let users of qemu_get_ram_ptr and qemu_ram_ptr_length pass in an
> address that is relative to the MemoryRegion. This basically means
> what address_space_translate returns.
>
> invalidate_and_set_dirty has to add back mr->ram_addr, but reads do
> not need it at all.
>
> Signed-off-by: Paolo Bonzini
I agree but I think we need a better name for this function.
qemu_ram_offset_to_ptr?
Will also serve to make sure backporting patches across this
API change does not cause issues.
> ---
> exec.c | 40 +++-
> include/exec/memory.h| 1 -
> memory.c | 4 ++--
> scripts/dump-guest-memory.py | 19 +++
> 4 files changed, 20 insertions(+), 44 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 001b669..ca9e3b6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1876,6 +1876,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t
> addr)
>
> if (block == NULL) {
> block = qemu_get_ram_block(addr);
> +addr -= block->offset;
> }
>
> if (xen_enabled() && block->host == NULL) {
> @@ -1889,7 +1890,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t
> addr)
>
> block->host = xen_map_cache(block->offset, block->max_length, 1);
> }
> -return ramblock_ptr(block, addr - block->offset);
> +return ramblock_ptr(block, addr);
> }
>
> /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
> @@ -1901,16 +1902,15 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block,
> ram_addr_t addr,
> hwaddr *size)
> {
> RAMBlock *block = ram_block;
> -ram_addr_t offset_inside_block;
> if (*size == 0) {
> return NULL;
> }
>
> if (block == NULL) {
> block = qemu_get_ram_block(addr);
> +addr -= block->offset;
> }
> -offset_inside_block = addr - block->offset;
> -*size = MIN(*size, block->max_length - offset_inside_block);
> +*size = MIN(*size, block->max_length - addr);
>
> if (xen_enabled() && block->host == NULL) {
> /* We need to check if the requested address is in the RAM
> @@ -1924,7 +1924,7 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block,
> ram_addr_t addr,
> block->host = xen_map_cache(block->offset, block->max_length, 1);
> }
>
> -return ramblock_ptr(block, offset_inside_block);
> +return ramblock_ptr(block, addr);
> }
>
> /*
> @@ -2504,6 +2504,8 @@ static void invalidate_and_set_dirty(MemoryRegion *mr,
> hwaddr addr,
> hwaddr length)
> {
> uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr);
> +addr += memory_region_get_ram_addr(mr);
> +
> /* No early return if dirty_log_mask is or becomes 0, because
> * cpu_physical_memory_set_dirty_range will still call
> * xen_modified_memory.
> @@ -2616,7 +2618,6 @@ static MemTxResult
> address_space_write_continue(AddressSpace *as, hwaddr addr,
> abort();
> }
> } else {
> -addr1 += memory_region_get_ram_addr(mr);
> /* RAM case */
> ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> memcpy(ptr, buf, l);
> @@ -2709,8 +2710,7 @@ MemTxResult address_space_read_continue(AddressSpace
> *as, hwaddr addr,
> }
> } else {
> /* RAM case */
> -ptr = qemu_get_ram_ptr(mr->ram_block,
> - memory_region_get_ram_addr(mr) + addr1);
> +ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> memcpy(buf, ptr, l);
> }
>
> @@ -2793,7 +2793,6 @@ static inline void
> cpu_physical_memory_write_rom_internal(AddressSpace *as,
>memory_region_is_romd(mr))) {
> l = memory_access_size(mr, l, addr1);
> } else {
> -addr1 += memory_region_get_ram_addr(mr);
> /* ROM/RAM case */
> ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> switch (type) {
> @@ -2953,7 +2952,6 @@ void *address_space_map(AddressSpace *as,
> hwaddr done = 0;
> hwaddr l, xlat, base;
> MemoryRegion *mr, *this_mr;
> -ram_addr_t raddr;
> void *ptr;
>
> if (len == 0) {
> @@ -2962,7 +2960,7 @@ void *address_space_map(AddressSpace *as,
>
> l = len;
> rcu_read_lock();
> -mr = address_space_translate(as, addr, &xlat, &l, is_write);
> +mr = address_space_translate(as, addr, &base, &l, is_write);
>
> if (!memory_access_is_direct(mr, is_write)) {
> if (atomic_xchg(&bounce.in_use, true)) {
> @@ -2987,9 +2985,6 @@ void *address_space_map(AddressSpace *as,
> return bounce.buffer;
> }
>
> -base = xlat;
> -raddr = memory_region_get_ram_addr(mr);
> -
> for (;;) {
> len -= l;
> addr += l;
> @@ -3007,7 +300
Re: [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr
On Thu, Mar 24, 2016 at 12:03:34PM +0100, Paolo Bonzini wrote:
> mr->ram_block->offset is already aligned to both host and target size
> (see qemu_ram_alloc_internal). Remove further masking as it is
> unnecessary.
>
> Signed-off-by: Paolo Bonzini
Reviewed-by: Michael S. Tsirkin
> ---
> exec.c | 21 +++--
> memory.c | 5 ++---
> migration/savevm.c | 4 ++--
> translate-all.c| 3 +--
> 4 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index f398d21..001b669 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1042,8 +1042,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
>
> if (memory_region_is_ram(section->mr)) {
> /* Normal RAM. */
> -iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK)
> -+ xlat;
> +iotlb = memory_region_get_ram_addr(section->mr) + xlat;
> if (!section->readonly) {
> iotlb |= PHYS_SECTION_NOTDIRTY;
> } else {
> @@ -3093,9 +3092,7 @@ static inline uint32_t
> address_space_ldl_internal(AddressSpace *as, hwaddr addr,
> } else {
> /* RAM case */
> ptr = qemu_get_ram_ptr(mr->ram_block,
> - (memory_region_get_ram_addr(mr)
> -& TARGET_PAGE_MASK)
> - + addr1);
> + memory_region_get_ram_addr(mr) + addr1);
> switch (endian) {
> case DEVICE_LITTLE_ENDIAN:
> val = ldl_le_p(ptr);
> @@ -3189,9 +3186,7 @@ static inline uint64_t
> address_space_ldq_internal(AddressSpace *as, hwaddr addr,
> } else {
> /* RAM case */
> ptr = qemu_get_ram_ptr(mr->ram_block,
> - (memory_region_get_ram_addr(mr)
> -& TARGET_PAGE_MASK)
> - + addr1);
> + memory_region_get_ram_addr(mr) + addr1);
> switch (endian) {
> case DEVICE_LITTLE_ENDIAN:
> val = ldq_le_p(ptr);
> @@ -3305,9 +3300,7 @@ static inline uint32_t
> address_space_lduw_internal(AddressSpace *as,
> } else {
> /* RAM case */
> ptr = qemu_get_ram_ptr(mr->ram_block,
> - (memory_region_get_ram_addr(mr)
> -& TARGET_PAGE_MASK)
> - + addr1);
> + memory_region_get_ram_addr(mr) + addr1);
> switch (endian) {
> case DEVICE_LITTLE_ENDIAN:
> val = lduw_le_p(ptr);
> @@ -3389,7 +3382,7 @@ void address_space_stl_notdirty(AddressSpace *as,
> hwaddr addr, uint32_t val,
>
> r = memory_region_dispatch_write(mr, addr1, val, 4, attrs);
> } else {
> -addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +addr1 += memory_region_get_ram_addr(mr);
> ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> stl_p(ptr, val);
>
> @@ -3444,7 +3437,7 @@ static inline void
> address_space_stl_internal(AddressSpace *as,
> r = memory_region_dispatch_write(mr, addr1, val, 4, attrs);
> } else {
> /* RAM case */
> -addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +addr1 += memory_region_get_ram_addr(mr);
> ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> switch (endian) {
> case DEVICE_LITTLE_ENDIAN:
> @@ -3554,7 +3547,7 @@ static inline void
> address_space_stw_internal(AddressSpace *as,
> r = memory_region_dispatch_write(mr, addr1, val, 2, attrs);
> } else {
> /* RAM case */
> -addr1 += memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK;
> +addr1 += memory_region_get_ram_addr(mr);
> ptr = qemu_get_ram_ptr(mr->ram_block, addr1);
> switch (endian) {
> case DEVICE_LITTLE_ENDIAN:
> diff --git a/memory.c b/memory.c
> index 95f7209..49c9b14 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1640,7 +1640,7 @@ int memory_region_get_fd(MemoryRegion *mr)
>
> assert(mr->ram_block);
>
> -return qemu_get_ram_fd(memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
> +return qemu_get_ram_fd(memory_region_get_ram_addr(mr));
> }
>
> void *memory_region_get_ram_ptr(MemoryRegion *mr)
> @@ -1654,8 +1654,7 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
> mr = mr->alias;
> }
> assert(mr->ram_block);
> -ptr = qemu_get_ram_ptr(mr->ram_block,
> - memory_region_get_ram_addr(mr) &
> TARGET_PAGE_MASK);
> +ptr = qemu_get_ram_ptr(mr->ram_block, memory_region_get_ram_addr(mr));
> rcu_read_unlock();
>
> return ptr + offset;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0a33c22..cbba062 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2229,13 +2229,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict
> *qdict)
>
Re: [Qemu-devel] [PATCH] acpi: Fix TPM ACPI description to make TPM usable on Windows
On Fri, Apr 01, 2016 at 02:50:43PM -0400, Stefan Berger wrote:
> On 03/31/2016 10:07 AM, Igor Mammedov wrote:
> >On Thu, 31 Mar 2016 00:03:57 -0400
> >Stefan Berger wrote:
> >
> >>On 03/30/2016 09:33 AM, Igor Mammedov wrote:
> >>>On Mon, 21 Mar 2016 10:21:11 -0400
> >>>Stefan Berger wrote:
> >>>
> This patch addresses BZ 1281413.
>
> Fix the APCI description to make it work on Windows again. The ACPI
> description was broken in commit 9e47226.
> >>>above commit just added missing ASL description for TMP device
> >>>and you also posted exactly similar patch back then.
> >>Sorry, I referenced the wrong commit. Here's the bad one:
> >>
> >>commit 72d97b3a543a9c2c820bd463ba24751ae4247ac3
> >>
> >>>Current commit message is pretty useless, Pls describe in commit
> >>>message what/how is broken and how/why patch fixes it.
> >>I am not sure what exactly broke it. All I know is that the scope was
> >>changed and the device name were change (ISA.TPM versus TPM). This
> >>was the working ACPI description from QEMU v2.3.1:
> >>
> >> Scope(\_SB) {
> >> /* TPM with emulated TPM TIS interface */
> >> Device (TPM) {
> >> Name (_HID, EisaID ("PNP0C31"))
> >> Name (_CRS, ResourceTemplate ()
> >> {
> >> Memory32Fixed (ReadWrite, TPM_TIS_ADDR_BASE,
> >>TPM_TIS_ADDR_SIZE)
> >> // older Linux tpm_tis drivers do not work with IRQ
> >> //IRQNoFlags () {TPM_TIS_IRQ}
> >> })
> >The first committed variant has IRQ uncommented, so it was always
> >present in _CRS (commit 9e47226)
> >
> >
> >[...]
> +//aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ));
> >>>silent change,
> >>>why IRQ descriptor is commented out, it seems the device
> >>>uses/initializes it?
> >>>I'd split IRQ change in a separate patch that explains why it
> >>>shouldn't be in TPM._CRS.
> >>We can leave it there if it works. The bug report is related to
> >>Windows, which I don't have running in a VM. So the safest was to
> >>roll back to 2.3.1 equivalent, which had the IRQ disabled.
> >I've looked through code some more and Windows seems to be right in not
> >starting device as IRQ 5 might be used by PNP0C0F devices, see
>
> On Linux I can use it with IRQ 5. I have tried with 10, it also works. But
> Linux != Windows.
>
>
> >build_link_dev(). So potential conflict is there and moving TPM device
> >out of PCI.ISA scope just hides problem as resource conflict
> >checking doesn't work anymore.
> >
> >Current TPM code have 2 issues:
> > 1: it uses wrong IRQ, (I've tried with unused COM2's IRQ3 and warning
> > goes away)
> > 2: IRQ is hardcoded i.e. _CRS always has TPM_TIS_IRQ value, regardless
> > of what user specified at command line, for example:
> >-device tpm-tis,irq=3
> > has no effect on ACPI part
>
> So if we disable interrupt, as done with this patch here, it works. This was
> the configuration we had in QEMU 2.3.1 and was reported as working here:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1281413
>
> So I would repost the patch with a better description of what we are rolling
> back. Let me know whether you agree. We can then enable the interrupt mode
> in separate patches.
I'm fine with a revert as a first step. But really we should avoid
asserting the interrupt for no-interrupt mode and obey what user
specified.
>
>
> >So to fix it correctly on top of my patch:
> > 1: default IRQ shouldn't be 5 (CCing Marcel, may be he has an idea
> > what it should be)
> > 2: ACPI should pick up IRQ from tpm-tis device property, i.e. it
> > shouldn't be hardcoded in ACPI part.
> >
Re: [Qemu-devel] [RFC] [tcg] Idea on refactoring target code generation loop (gen_intermediate_code)
Markus Armbruster writes: > Peter Maydell writes: > [...] >> if we move away from C I'd rather >> it to be a language that's nicer than C rather than one that's >> uglier and larger and still retains all of C's flaws. > Seconded strongly. Just curious. Do we agree though that moving selected pieces like the code templates with macros to C++ templated functions, or QOM to C++ classes would be an improvement over the current C code without any dramatic changes in the codebase? (I don't think more than that is needed) Cheers, Lluis
Re: [Qemu-devel] segmentation fault in object.c:type_initialize_interface() if interface not defined
On 2 April 2016 at 23:15, Liviu Ionescu wrote: > I just updated GNU ARM Eclipse QEMU to 2.5.1 and initially >I had some problems, main() failed quite early, in the first > call to `find_default_machine()`. > > After several debug sessions, I identified the problem to > be a null pointer when a referred interface is not defined. > In my Cortex-M specific configuration, `arm/boot.c` was not > included in the build, but TYPE_ARM_LINUX_BOOT_IF was referred > by TYPE_ARM_GIC_COMMON, the parent of my NVIC object. > > I guess the problem is in `object.c:type_initialize()`, which > does not check the pointer returned by: > > `TypeImpl *t = type_get_by_name(ti->interfaces[i].typename)` > > and calls > > `type_initialize_interface(ti, t, t);` > > with the null pointers. Yeah, referring to an interface that doesn't exist is a program bug (or in this case a build config error, though since hw/arm/boot.o is in obj-y it should always be built), but we could assert on it rather than just crashing. g_assert() will do. thanks -- PMM
Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix sending mouse event to guest
On 2 April 2016 at 18:53, Programmingkid wrote:
>
> On Apr 2, 2016, at 1:35 PM, Peter Maydell wrote:
>
>> On 2 April 2016 at 18:25, Programmingkid wrote:
>>>
>>> On Apr 2, 2016, at 1:07 PM, Peter Maydell wrote:
>>>
On 2 April 2016 at 17:56, Programmingkid wrote:
> The mouse down event should not be sent to the guest if the mouse down
> event
> causes an activation of QEMU. This patch prevents activation clicks from
> going
> to the guest.
>
> Signed-off-by: John Arbuckle
> ---
> ui/cocoa.m | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 60a7c07..07d9c86 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -698,7 +698,7 @@ QemuCocoaView *cocoaView;
> * call below. We definitely don't want to pass that click through
> * to the guest.
> */
> -if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
> +if ((isMouseGrabbed && [[self window] isKeyWindow]) &&
>(last_buttons != buttons)) {
>static uint32_t bmap[INPUT_BUTTON__MAX] = {
>[INPUT_BUTTON_LEFT] = MOUSE_EVENT_LBUTTON,
> --
> 2.7.2
I'm afraid I don't really understand why you think this
should change. On the face of it the current code looks right:
we pass through the mouse button if:
(1) we've got the mouse grab
or (2) our window has the focus, even if it's not grabbed
I would expect the "activation click" to be "we don't have
the mouse grab, and we don't have focus either (some other
app is foreground)".
>>>
>>> When QEMU's main window is in the background and the user clicks on it,
>>> the NSLeftMouseUp case in the handleEvent: method will set the
>>> isMouseGrabbed variable to true. This means the
>>> "if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
>>> (last_buttons != buttons))" code will always be true for a left
>>> mouse button click. The mouse event will be sent to the guest
>>> when it shouldn't be.
>>
>> OK, that sounds like a bug, but this doesn't look like the
>> right way to fix it, because with your change we won't
>> pass through the click if this is a click on the window
>> when it's not in the background.
>
> I think logically this is the best solution. If the window is in
> the foreground and doesn't have the mouse grabbed, the user can't
> move the mouse pointer in the guest. This could cause stuff to be
> clicked that might cause undesirable effects.
So why do we now have a different condition for "don't pass
through buttons" and "don't pass through mouse events" ?
The process for making changes should be something like:
* identify the model we are trying to use to decide whether
to pass events through to the guest or not
* if this model is wrong (ie there is a design flaw),
identify what the changes to the design need to be
(possibly looking at other OSX VM UIs and how we handle
similar cases in our other UI frontends, to identify what
might be reasonable behaviour), so that we have a coherent
design which we can use now and in the future to fix the code
* work out where the code is diverging from the design
(either because the code is buggy or because we made a
change to the design in the previous step)
* change the code
* make sure that comments match the changed code/design
I don't have strong feelings about what the right design for
handling mouse events should be, but I do think we should
have one, and this patch seems to be making a local change
rather than considering the larger picture.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2] firmware: qemu_fw_cfg.c: hold ACPI global lock during device access
On Tue, Mar 08, 2016 at 01:30:50PM -0500, Gabriel Somlo wrote:
> Allowing for the future possibility of implementing AML-based
> (i.e., firmware-triggered) access to the QEMU fw_cfg device,
> acquire the global ACPI lock when accessing the device on behalf
> of the guest-side sysfs driver, to prevent any potential race
> conditions.
>
> Suggested-by: Michael S. Tsirkin
> Signed-off-by: Gabriel Somlo
Greg, could you merge this please?
I'm rather worried that this goes out without.
> ---
>
> Changes since v1:
> - no more "#ifdef CONFIG_ACPI"; instead we proceed if
> acpi_acquire_global_lock() returns either OK or NOT_CONFIGURED,
> and only throw a warning/error message otherwise.
>
> - didn't get any *negative* feedback from the QEMU crowd, so
> this is now a bona-fide "please apply this", rather than just
> an RFC :)
>
> - tested on ACPI-enabled x86_64, and acpi_less ARM (32 and 64 bit)
> QEMU VMs (I don't have handy access to an ACPI-enabled ARM VM)
>
> Thanks much,
> --Gabriel
>
> drivers/firmware/qemu_fw_cfg.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 7bba76c..a44dc32 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -77,12 +77,28 @@ static inline u16 fw_cfg_sel_endianness(u16 key)
> static inline void fw_cfg_read_blob(u16 key,
> void *buf, loff_t pos, size_t count)
> {
> + u32 glk;
> + acpi_status status;
> +
> + /* If we have ACPI, ensure mutual exclusion against any potential
> + * device access by the firmware, e.g. via AML methods:
> + */
> + status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, &glk);
> + if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> + /* Should never get here */
> + WARN(1, "fw_cfg_read_blob: Failed to lock ACPI!\n");
> + memset(buf, 0, count);
> + return;
> + }
> +
> mutex_lock(&fw_cfg_dev_lock);
> iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
> while (pos-- > 0)
> ioread8(fw_cfg_reg_data);
> ioread8_rep(fw_cfg_reg_data, buf, count);
> mutex_unlock(&fw_cfg_dev_lock);
> +
> + acpi_release_global_lock(glk);
> }
>
> /* clean up fw_cfg device i/o */
> --
> 2.4.3
[Qemu-devel] [PATCH] virtio-blk: assert on starting/stopping
Reentrancy cannot happen while the BQL is being held,
so we should never enter this condition.
Cc: Christian Borntraeger
Cc: Cornelia Huck
Cc: Paolo Bonzini
Signed-off-by: Michael S. Tsirkin
---
This is a replacement for [PATCH 9/9] virtio: remove starting/stopping
checks Christian, could you please give it a spin with debug enabled?
Since you reported above Paolo's patch triggers segfaults, I expect this
one to trigger assertions as well, which should give us more info on
the root cause.
hw/block/dataplane/virtio-blk.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index fd06726..04e0e0d 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -203,10 +203,12 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
int r;
-if (vblk->dataplane_started || s->starting) {
+if (vblk->dataplane_started) {
return;
}
+assert(!s->starting);
+
s->starting = true;
s->vq = virtio_get_queue(s->vdev, 0);
@@ -257,10 +259,12 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
-if (!vblk->dataplane_started || s->stopping) {
+if (!vblk->dataplane_started) {
return;
}
+assert(!s->stopping);
+
/* Better luck next time. */
if (vblk->dataplane_disabled) {
vblk->dataplane_disabled = false;
--
MST
Re: [Qemu-devel] [PATCH 9/9] virtio: remove starting/stopping checks
On Fri, Apr 01, 2016 at 04:30:44PM +0200, Cornelia Huck wrote: > On Fri, 1 Apr 2016 16:14:22 +0200 > Christian Borntraeger wrote: > > > On 04/01/2016 03:19 PM, Paolo Bonzini wrote: > > > Reentrancy cannot happen while the BQL is being held. > > > > > > Reviewed-by: Cornelia Huck > > > Signed-off-by: Paolo Bonzini > > > > Reverting this patch makes the segfaults go away. > > > > > > > > > > > > > --- > > > hw/block/dataplane/virtio-blk.c | 12 ++-- > > > hw/scsi/virtio-scsi-dataplane.c | 9 + > > > include/hw/virtio/virtio-scsi.h | 2 -- > > > 3 files changed, 3 insertions(+), 20 deletions(-) > > :( > > On the one hand, I'm wondering what we're missing here. > > On the other hand, let's just skip the cleanup patches and get the bug > fixed? For 2.6, absolutely. I would also drop 8/9. For debugging (that we can keep up for now), instead of dropping the checks, let's replace them with asserts. -- MST
Re: [Qemu-devel] [PATCH v2 0/9] virtio: aio handler API
On Fri, Apr 01, 2016 at 03:19:45PM +0200, Paolo Bonzini wrote: > This version fixes some commit messages, is based on qemu.git master > and adds Cornelia's Reviewed-by tags. There are no code changes apart > from context. I agree with patches 1-7 for 2.6. 8 and 9 are cleanups and IMO they are best left for after 2.6. Which is all nice and well, but we need to get to the bottom of the crashes reported by Christian. > Michael S. Tsirkin (2): > virtio: add aio handler > virtio-blk: use aio handler for data plane > > Paolo Bonzini (7): > virtio-dataplane: pass assign=true to > virtio_queue_aio_set_host_notifier_handler > virtio: make virtio_queue_notify_vq static > virtio-blk: fix disabled mode > virtio-scsi: fix disabled mode > virtio-scsi: use aio handler for data plane > virtio: merge virtio_queue_aio_set_host_notifier_handler with > virtio_queue_set_aio > virtio: remove starting/stopping checks > > hw/block/dataplane/virtio-blk.c | 35 +++-- > hw/block/virtio-blk.c | 29 ++--- > hw/scsi/virtio-scsi-dataplane.c | 56 +++-- > hw/scsi/virtio-scsi.c | 69 > +++-- > hw/virtio/virtio.c | 37 -- > include/hw/virtio/virtio-blk.h | 3 ++ > include/hw/virtio/virtio-scsi.h | 9 ++ > include/hw/virtio/virtio.h | 4 +-- > 8 files changed, 158 insertions(+), 84 deletions(-) > > -- > 1.8.3.1
Re: [Qemu-devel] [PATCH 8/9] virtio: merge virtio_queue_aio_set_host_notifier_handler with virtio_queue_set_aio
On Fri, Apr 01, 2016 at 03:19:53PM +0200, Paolo Bonzini wrote:
> Eliminating the reentrancy is actually a nice thing that we can do
> with the API that Michael proposed, so let's make it first class.
> This also hides the complex assign/set_handler conventions from
> callers of virtio_queue_aio_set_host_notifier_handler, which in
> fact was always called with assign=true.
>
> Reviewed-by: Cornelia Huck
> Signed-off-by: Paolo Bonzini
> ---
> hw/block/dataplane/virtio-blk.c | 7 +++
> hw/scsi/virtio-scsi-dataplane.c | 12
> hw/virtio/virtio.c | 19 ---
> include/hw/virtio/virtio.h | 6 ++
> 4 files changed, 13 insertions(+), 31 deletions(-)
>
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index fd06726..74c6d37 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -237,8 +237,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
>
> /* Get this show started by hooking up our callbacks */
> aio_context_acquire(s->ctx);
> -virtio_set_queue_aio(s->vq, virtio_blk_data_plane_handle_output);
> -virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true);
> +virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx,
> +
> virtio_blk_data_plane_handle_output);
> aio_context_release(s->ctx);
> return;
>
> @@ -273,8 +273,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
> aio_context_acquire(s->ctx);
>
> /* Stop notifications for new requests from guest */
> -virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, false);
> -virtio_set_queue_aio(s->vq, NULL);
> +virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, NULL);
>
> /* Drain and switch bs back to the QEMU main loop */
> blk_set_aio_context(s->conf->conf.blk, qemu_get_aio_context());
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index a497a2c..5494dcc 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -81,8 +81,7 @@ static int virtio_scsi_vring_init(VirtIOSCSI *s, VirtQueue
> *vq, int n,
> return rc;
> }
>
> -virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, true, true);
> -virtio_set_queue_aio(vq, fn);
> +virtio_queue_aio_set_host_notifier_handler(vq, s->ctx, fn);
> return 0;
> }
>
> @@ -99,13 +98,10 @@ static void virtio_scsi_clear_aio(VirtIOSCSI *s)
> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> int i;
>
> -virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, true,
> false);
> -virtio_set_queue_aio(vs->ctrl_vq, NULL);
> -virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, true,
> false);
> -virtio_set_queue_aio(vs->event_vq, NULL);
> +virtio_queue_aio_set_host_notifier_handler(vs->ctrl_vq, s->ctx, NULL);
> +virtio_queue_aio_set_host_notifier_handler(vs->event_vq, s->ctx, NULL);
> for (i = 0; i < vs->conf.num_queues; i++) {
> -virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
> true, false);
> -virtio_set_queue_aio(vs->cmd_vqs[i], NULL);
> +virtio_queue_aio_set_host_notifier_handler(vs->cmd_vqs[i], s->ctx,
> NULL);
> }
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index eb04ac0..7fcfc24f 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1159,14 +1159,6 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
> queue_size,
> return &vdev->vq[i];
> }
>
> -void virtio_set_queue_aio(VirtQueue *vq,
> - void (*handle_output)(VirtIODevice *, VirtQueue *))
> -{
> -assert(vq->handle_output);
> -
> -vq->handle_aio_output = handle_output;
> -}
> -
> void virtio_del_queue(VirtIODevice *vdev, int n)
> {
> if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
> @@ -1809,19 +1801,16 @@ static void
> virtio_queue_host_notifier_aio_read(EventNotifier *n)
> }
>
> void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext
> *ctx,
> -bool assign, bool
> set_handler)
> +void
> (*handle_output)(VirtIODevice *,
> +VirtQueue *))
> {
> -if (assign && set_handler) {
> +vq->handle_aio_output = handle_output;
> +if (handle_output) {
> aio_set_event_notifier(ctx, &vq->host_notifier, true,
> virtio_queue_host_notifier_aio_read);
> } else {
> aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL);
> }
> -if (!assign) {
> -/* Test and clear notifier before after disabling event,
> - * in case poll callback didn't have time to run. */
> -virtio_queue_host_notifier_aio_read(&vq->host_notifier);
> -}
> }
>
> static void
Re: [Qemu-devel] [PATCH v2 0/9] virtio: aio handler API
On Fri, Apr 01, 2016 at 05:16:06PM +0200, Christian Borntraeger wrote: > Now with enable-debug and better call traces > > (gdb) > (gdb) thread apply all bt > > Thread 5 (Thread 0x3ffa0e7f910 (LWP 29839)): > #0 0x03ffa530334a in ioctl () at /lib64/libc.so.6 > #1 0x80081c84 in kvm_vcpu_ioctl (cpu=0x80e8c170, type=44672) at > /home/cborntra/REPOS/qemu/kvm-all.c:1984 > #2 0x8008154c in kvm_cpu_exec (cpu=0x80e8c170) at > /home/cborntra/REPOS/qemu/kvm-all.c:1834 > #3 0x8006075c in qemu_kvm_cpu_thread_fn (arg=0x80e8c170) at > /home/cborntra/REPOS/qemu/cpus.c:1056 > #4 0x03ffa5407c2c in start_thread () at /lib64/libpthread.so.0 > #5 0x03ffa530ec9a in thread_start () at /lib64/libc.so.6 > > Thread 4 (Thread 0x3ffa3c7f910 (LWP 29830)): > #0 0x03ffa530841e in syscall () at /lib64/libc.so.6 > #1 0x803e83fe in futex_wait (ev=0x80a65bd4 , > val=4294967295) at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:292 > #2 0x803e868e in qemu_event_wait (ev=0x80a65bd4 > ) at > /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:399 > #3 0x80405dcc in call_rcu_thread (opaque=0x0) at > /home/cborntra/REPOS/qemu/util/rcu.c:250 > #4 0x03ffa5407c2c in start_thread () at /lib64/libpthread.so.0 > #5 0x03ffa530ec9a in thread_start () at /lib64/libc.so.6 > > Thread 3 (Thread 0x3ffa16c3910 (LWP 29838)): > #0 0x03ffa5410cd4 in __lll_lock_wait () at /lib64/libpthread.so.0 > #1 0x03ffa5413ed4 in __lll_lock_elision () at /lib64/libpthread.so.0 > #2 0x803e7ace in qemu_mutex_lock (mutex=0x8063ad08 > ) at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:64 > #3 0x80060f04 in qemu_mutex_lock_iothread () at > /home/cborntra/REPOS/qemu/cpus.c:1232 > #4 0x80158866 in kvm_arch_handle_exit (cs=0x80e4d2b0, > run=0x3ffa0e8) at /home/cborntra/REPOS/qemu/target-s390x/kvm.c:2024 > #5 0x8008181e in kvm_cpu_exec (cpu=0x80e4d2b0) at > /home/cborntra/REPOS/qemu/kvm-all.c:1921 > #6 0x8006075c in qemu_kvm_cpu_thread_fn (arg=0x80e4d2b0) at > /home/cborntra/REPOS/qemu/cpus.c:1056 > #7 0x03ffa5407c2c in start_thread () at /lib64/libpthread.so.0 > #8 0x03ffa530ec9a in thread_start () at /lib64/libc.so.6 > > Thread 2 (Thread 0x3ffa6edbb90 (LWP 29795)): > #0 0x03ffa540d68a in pthread_cond_wait@@GLIBC_2.3.2 () at > /lib64/libpthread.so.0 > #1 0x803e7d66 in qemu_cond_wait (cond=0x80acd660, mutex=0x80acd630) > at /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:123 > #2 0x80405618 in rfifolock_lock (r=0x80acd630) at > /home/cborntra/REPOS/qemu/util/rfifolock.c:59 > #3 0x802dfafa in aio_context_acquire (ctx=0x80acd5d0) at > /home/cborntra/REPOS/qemu/async.c:373 > #4 0x802eb7e0 in bdrv_set_aio_context (bs=0x80b00d30, > new_context=0x80acd5d0) at /home/cborntra/REPOS/qemu/block.c:3682 > #5 0x8034834e in blk_set_aio_context (blk=0x80af81f0, > new_context=0x80acd5d0) at > /home/cborntra/REPOS/qemu/block/block-backend.c:1371 > #6 0x800b7b08 in virtio_blk_data_plane_start (s=0x80f39530) at > /home/cborntra/REPOS/qemu/hw/block/dataplane/virtio-blk.c:228 So we are in virtio_blk_data_plane_start, but before we set aio handler. > #7 0x800b57ba in virtio_blk_handle_output (vdev=0x80f18008, > vq=0x80fa25d0) at /home/cborntra/REPOS/qemu/hw/block/virtio-blk.c:607 > #8 0x800f0c7c in virtio_queue_notify_vq (vq=0x80fa25d0) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1108 > #9 0x800f3674 in virtio_queue_host_notifier_read (n=0x80fa2630) at > /home/cborntra/REPOS/qemu/hw/virtio/virtio.c:1820 > #10 0x802f1914 in aio_dispatch (ctx=0x80abae30) at > /home/cborntra/REPOS/qemu/aio-posix.c:327 > #11 0x802df3dc in aio_ctx_dispatch (source=0x80abae30, callback=0x0, > user_data=0x0) at /home/cborntra/REPOS/qemu/async.c:233 > #12 0x03ffa5c51c0a in g_main_context_dispatch () at > /lib64/libglib-2.0.so.0 > #13 0x802ee616 in glib_pollfds_poll () at > /home/cborntra/REPOS/qemu/main-loop.c:213 > #14 0x802ee752 in os_host_main_loop_wait (timeout=132800) at > /home/cborntra/REPOS/qemu/main-loop.c:258 > #15 0x802ee85e in main_loop_wait (nonblocking=0) at > /home/cborntra/REPOS/qemu/main-loop.c:506 > #16 0x8017db14 in main_loop () at /home/cborntra/REPOS/qemu/vl.c:1934 > #17 0x80185fe8 in main (argc=72, argv=0x3ffd157ea28, > envp=0x3ffd157ec70) at /home/cborntra/REPOS/qemu/vl.c:4652 > > Thread 1 (Thread 0x3ffa347f910 (LWP 29831)): > #0 0x03ffa540a178 in pthread_mutex_lock () at /lib64/libpthread.so.0 > #1 0x803e7ace in qemu_mutex_lock (mutex=0xf0) at > /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:64 > #2 0x802deba8 in aio_bh_new (ctx=0x0, cb=0x80346910 > , opaque=0x3ff9c000a50) at > /home/cborntra/REPOS/qemu/async.c:55 > #3 0x80346a6e in blk_aio_prwv (blk=0x80af81f0, offset=4096, > qiov=0x3ff9c0021c8, co_entry=0x80346aa8
[Qemu-devel] [PATCH] xen: piix reuse pci generic class init function
piix3_ide_xen_class_init is identical to piix3_ide_class_init
except it's buggy as it does not set exit and does not disable
hotplug properly.
Switch to the generic one.
Reviewed-by: Stefano Stabellini
Signed-off-by: Michael S. Tsirkin
---
hw/ide/piix.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index df46147..0a4cbcb 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -258,22 +258,10 @@ static const TypeInfo piix3_ide_info = {
.class_init= piix3_ide_class_init,
};
-static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-k->realize = pci_piix_ide_realize;
-k->vendor_id = PCI_VENDOR_ID_INTEL;
-k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
-k->class_id = PCI_CLASS_STORAGE_IDE;
-set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-}
-
static const TypeInfo piix3_ide_xen_info = {
.name = "piix3-ide-xen",
.parent= TYPE_PCI_IDE,
-.class_init= piix3_ide_xen_class_init,
+.class_init= piix3_ide_class_init,
};
static void piix4_ide_class_init(ObjectClass *klass, void *data)
--
MST
