date:20180911

On Tue, Sep 11, 2018 at 11:49:49AM -0500, Brijesh Singh wrote:
> Now that amd-iommu support interrupt remapping, enable the GASup in IVRS
> table and GASup in extended feature register to indicate that IOMMU
> support guest virtual APIC mode.
> 
> Note that the GAMSup is set to zero to indicate that  Guest Virtual
> APIC does not support advanced interrupt features (i.e virtualized
> interrupts using the guest virtual APIC).
> 
> See Table 21 from IOMMU spec for interrupt virtualization controls
> 
> IOMMU spec: https://support.amd.com/TechDocs/48882_IOMMU.pdf
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcel Apfelbaum 
> Cc: Tom Lendacky 
> Cc: Suravee Suthikulpanit 
> Signed-off-by: Brijesh Singh 
> ---
>  hw/i386/acpi-build.c | 3 ++-
>  hw/i386/amd_iommu.h  | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5c2c638..1cbc8ba 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2565,7 +2565,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
>  build_append_int_noprefix(table_data,
>   (48UL << 30) | /* HATS   */
>   (48UL << 28) | /* GATS   */
> - (1UL << 2),/* GTSup  */
> + (1UL << 2)   | /* GTSup  */
> + (1UL << 6),/* GASup  */

Sorry if I misunderstood - is this for nested?

I'm a bit confused here... IIUC in your previous patches you didn't
really implement guest_mode==1 case in IRTEs.  So if you have this set
then the guest should be able to setup IRTEs with guest_mode==1?  How
did it work?

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 4/6] i386: acpi: add IVHD device entry for IOAPIC

On Tue, Sep 11, 2018 at 11:49:47AM -0500, Brijesh Singh wrote:
> When interrupt remapping is enabled, add a special IVHD device
> (type IOAPIC) -- which is typically PCI device 14:0.0. Linux IOMMU driver
> checks for this special device.
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcel Apfelbaum 
> Cc: Tom Lendacky 
> Cc: Suravee Suthikulpanit 
> Signed-off-by: Brijesh Singh 
> ---
>  hw/i386/acpi-build.c | 20 +++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e1ee8ae..5c2c638 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2519,6 +2519,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
>  static void
>  build_amd_iommu(GArray *table_data, BIOSLinker *linker)
>  {
> +int ivhd_table_len = 28;
>  int iommu_start = table_data->len;
>  AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
>  
> @@ -2540,8 +2541,16 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
>   (1UL << 6) | /* PrefSup  */
>   (1UL << 7),  /* PPRSup   */
>   1);
> +
> +/*
> + * When interrupt remapping is enabled, we add a special IVHD device
> + * for type IO-APIC.
> + */
> +if (s->intr_enabled) {
> +ivhd_table_len += 8;
> +}
>  /* IVHD length */
> -build_append_int_noprefix(table_data, 28, 2);
> +build_append_int_noprefix(table_data, ivhd_table_len, 2);
>  /* DeviceID */
>  build_append_int_noprefix(table_data, s->devid, 2);
>  /* Capability offset */
> @@ -2565,6 +2574,15 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
>   */
>  build_append_int_noprefix(table_data, 0x001, 4);
>  
> +/*
> + * When interrupt remapping is enabled, Linux IOMMU driver also checks
> + * for special IVHD device (type IO-APIC), which is typically presented
> + * as PCI device 14:00.0.
> + */
> +if (s->intr_enabled) {
> +build_append_int_noprefix(table_data, 0x0100a048, 8);

Some comments on the bit definition would be nicer, or "please refer
to Table 95 of AMD-Vi spec".

Could I ask how come the 14:00.0?  Is that in the spec somewhere?

And since you explicitly mentioned Linux, then... would it work for
Windows too?

Thanks,

> +}
> +
>  build_header(linker, table_data, (void *)(table_data->data + 
> iommu_start),
>   "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
>  }
> -- 
> 2.7.4
> 
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 1/6] x86_iommu: move the kernel-irqchip check in common code

On Tue, Sep 11, 2018 at 11:49:44AM -0500, Brijesh Singh wrote:
> Interrupt remapping needs kernel-irqchip={off|split} on both Intel and AMD
> platforms. Move the check in common place.
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcel Apfelbaum 
> Cc: Tom Lendacky 
> Cc: Suravee Suthikulpanit 
> Signed-off-by: Brijesh Singh 

Reviewed-by: Peter Xu 

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 2/6] x86_iommu/amd: Prepare for interrupt remap support

On Tue, Sep 11, 2018 at 11:49:45AM -0500, Brijesh Singh wrote:
>  static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int 
> devfn)
>  {
>  AMDVIState *s = opaque;
> @@ -1055,6 +1151,12 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
> void *opaque, int devfn)
>  address_space_init(&iommu_as[devfn]->as,
> MEMORY_REGION(&iommu_as[devfn]->iommu),
> "amd-iommu");
> +memory_region_init_io(&iommu_as[devfn]->iommu_ir, OBJECT(s),
> +  &amdvi_ir_ops, s, "amd-iommu-ir",
> +  AMDVI_INT_ADDR_SIZE);
> +memory_region_add_subregion(MEMORY_REGION(&iommu_as[devfn]->iommu),
> +  AMDVI_INT_ADDR_FIRST,
> +  &iommu_as[devfn]->iommu_ir);

A pure question: just to make sure this IR region won't be masked out
by other memory regions.  Asked since VT-d is explicitly setting a
higher priority of the memory region for interrupts with
memory_region_add_subregion_overlap().

>  }
>  return &iommu_as[devfn]->as;
>  }
> @@ -1172,6 +1274,10 @@ static void amdvi_realize(DeviceState *dev, Error 
> **err)
>  return;
>  }
>  
> +/* Pseudo address space under root PCI bus. */
> +pcms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_SB_IOAPIC_ID);
> +s->intr_enabled = x86_iommu->intr_supported;

So does this mean that AMD IR cannot be disabled if declared support?
For VT-d, IR needs to be explicitly enabled otherwise disabled (even
supported).

Otherwise the patch looks good to me.  Thanks,

> +
>  /* set up MMIO */
>  memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, 
> "amdvi-mmio",
>AMDVI_MMIO_SIZE);
> @@ -1205,6 +1311,7 @@ static void amdvi_class_init(ObjectClass *klass, void* 
> data)
>  dc->vmsd = &vmstate_amdvi;
>  dc->hotpluggable = false;
>  dc_class->realize = amdvi_realize;
> +dc_class->int_remap = amdvi_int_remap;
>  /* Supported by the pc-q35-* machine types */
>  dc->user_creatable = true;
>  }

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 3/6] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled

On Tue, Sep 11, 2018 at 11:49:46AM -0500, Brijesh Singh wrote:
> Emulate the interrupt remapping support when guest virtual APIC is
> not enabled.
> 
> See IOMMU spec: https://support.amd.com/TechDocs/48882_IOMMU.pdf
> (section 2.2.5.1) for details information.
> 
> When VAPIC is not enabled, it uses interrupt remapping as defined in
> Table 20 and Figure 15 from IOMMU spec.
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcel Apfelbaum 
> Cc: Tom Lendacky 
> Cc: Suravee Suthikulpanit 
> Signed-off-by: Brijesh Singh 
> ---
>  hw/i386/amd_iommu.c  | 187 
> +++
>  hw/i386/amd_iommu.h  |  60 -
>  hw/i386/trace-events |   7 ++
>  3 files changed, 253 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 572ba0a..5ac19df 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -28,6 +28,8 @@
>  #include "qemu/error-report.h"
>  #include "hw/i386/apic_internal.h"
>  #include "trace.h"
> +#include "cpu.h"
> +#include "hw/i386/apic-msidef.h"
>  
>  /* used AMD-Vi MMIO registers */
>  const char *amdvi_mmio_low[] = {
> @@ -1027,6 +1029,119 @@ static IOMMUTLBEntry 
> amdvi_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>  return ret;
>  }
>  
> +static int amdvi_get_irte(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
> +  union irte *irte, uint16_t devid)
> +{
> +uint64_t irte_root, offset;
> +
> +irte_root = dte[2] & AMDVI_IR_PHYS_ADDR_MASK;
> +offset = (origin->data & AMDVI_IRTE_OFFSET) << 2;
> +
> +trace_amdvi_ir_irte(irte_root, offset);
> +
> +if (dma_memory_read(&address_space_memory, irte_root + offset,
> +irte, sizeof(*irte))) {
> +trace_amdvi_ir_err("failed to get irte");
> +return -AMDVI_IR_GET_IRTE;
> +}
> +
> +trace_amdvi_ir_irte_val(irte->val);
> +
> +return 0;
> +}
> +
> +static void amdvi_generate_msi_message(struct AMDVIIrq *irq, MSIMessage *out)
> +{
> +out->address = APIC_DEFAULT_ADDRESS | \
> +(irq->dest_mode << MSI_ADDR_DEST_MODE_SHIFT) | \
> +(irq->redir_hint << MSI_ADDR_REDIRECTION_SHIFT) | \
> +(irq->dest << MSI_ADDR_DEST_ID_SHIFT);
> +
> +out->data = (irq->vector << MSI_DATA_VECTOR_SHIFT) | \
> +(irq->delivery_mode << MSI_DATA_DELIVERY_MODE_SHIFT);
> +
> +trace_amdvi_ir_generate_msi_message(irq->vector, irq->delivery_mode,
> +irq->dest_mode, irq->dest, irq->redir_hint);
> +}
> +
> +static int amdvi_int_remap_legacy(AMDVIState *iommu,
> +  MSIMessage *origin,
> +  MSIMessage *translated,
> +  uint64_t *dte,
> +  struct AMDVIIrq *irq,
> +  uint16_t sid)
> +{
> +int ret;
> +union irte irte;
> +
> +/* get interrupt remapping table */

... get interrupt remapping table "entry"? :)

I see similar wordings in your spec, e.g., Table 20 is named as
"Interrupt Remapping Table Fields - Basic Format", but actually AFAICT
it's for the entry fields.  I'm confused a bit with them.

> +ret = amdvi_get_irte(iommu, origin, dte, &irte, sid);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +if (!irte.fields.valid) {
> +trace_amdvi_ir_target_abort("RemapEn is disabled");
> +return -AMDVI_IR_TARGET_ABORT;
> +}
> +
> +if (irte.fields.guest_mode) {
> +trace_amdvi_ir_target_abort("guest mode is not zero");
> +return -AMDVI_IR_TARGET_ABORT;
> +}
> +
> +if (irte.fields.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
> +trace_amdvi_ir_target_abort("reserved int_type");
> +return -AMDVI_IR_TARGET_ABORT;
> +}
> +
> +irq->delivery_mode = irte.fields.int_type;
> +irq->vector = irte.fields.vector;
> +irq->dest_mode = irte.fields.dm;
> +irq->redir_hint = irte.fields.rq_eoi;
> +irq->dest = irte.fields.destination;
> +
> +return 0;
> +}
> +
> +static int __amdvi_int_remap_msi(AMDVIState *iommu,
> + MSIMessage *origin,
> + MSIMessage *translated,
> + uint64_t *dte,
> + uint16_t sid)
> +{
> +int ret;
> +uint8_t int_ctl;
> +struct AMDVIIrq irq = { 0 };
> +
> +int_ctl = (dte[2] >> AMDVI_IR_INTCTL_SHIFT) & 3;
> +trace_amdvi_ir_intctl(int_ctl);
> +
> +switch (int_ctl) {
> +case AMDVI_IR_INTCTL_PASS:
> +memcpy(translated, origin, sizeof(*origin));
> +return 0;
> +case AMDVI_IR_INTCTL_REMAP:
> +break;
> +case AMDVI_IR_INTCTL_ABORT:
> +trace_amdvi_ir_target_abort("int_ctl abort");
> +return -AMDVI_IR_TARGET_ABORT;
> +default:
> +trace_amdvi_ir_target_abort("int_ctl reserved");
> +return -AMDVI_IR_TARGET_ABOR

Re: [Qemu-devel] [PATCH v4 0/3] x86: QEMU side support on MSR based features

2018-09-11 Thread Robert Hoo

On Sun, 2018-09-02 at 19:46 +0800, Robert Hoo wrote:
Ping ... :-)
> KVM side has added the framework (kvm.git:d1d93fa90) to support MSR
> based features.
> Here is the QEMU part, including data structure changes/expanding,
> referring
> functions changes, and the implementations on 
> KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl.
> 
> Changelog:
> v4:
>   Re-organize patch set to conform to request of individually
> build pass.
>   Add KVM capability check for KVM_GET_MSR_INDEX_LIST before
> fetch.
>   Special treatment for MSR_IA32_ARCH_CAPABILITIES.RSBA.
>   Use more convenient glib wrapper (g_strdup_printf) instead of
> native
> (sprintf).
>   
> v3: patch 2&3 in v2 are corrupted. Re-format patches.
> v2: coding style changes to pass ./scripts/checkpatch.pl.
> 
> Robert Hoo (3):
>   x86: Data structure changes to support MSR based features
>   kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and 
> KVM_GET_MSRS system ioctl
>   x86: define a new MSR based feature word --
> FEATURE_WORDS_ARCH_CAPABILITIES
> 
>  include/sysemu/kvm.h |   2 +
>  target/i386/cpu.c| 200 +--
> 
>  target/i386/cpu.h|  12 
>  target/i386/kvm.c|  72 +++
>  4 files changed, 233 insertions(+), 53 deletions(-)
>

Re: [Qemu-devel] [PATCH v2 12/12] qht-bench: add -p flag to precompute hash values

On Tue, Sep 11, 2018 at 17:46:41 -0700, Richard Henderson wrote:
> On 09/10/2018 11:58 AM, Emilio G. Cota wrote:
> > @@ -289,7 +297,9 @@ static void htable_init(void)
> >  /* avoid allocating memory later by allocating all the keys now */
> >  keys = g_malloc(sizeof(*keys) * n);
> >  for (i = 0; i < n; i++) {
> > -keys[i] = populate_offset + i;
> > +long val = populate_offset + i;
> > +
> > +keys[i] = precompute_hash ? h(val) : hval(val);
> 
> hfunc?

Here is where precomputation happens, so if precompute_hash is set,
then we insert the hashed value. Otherwise we insert the non-hashed
value (with hval()). In all other instances we use hfunc, since
hfunc is set to the "other" hash function wrt the above--see
this later hunk:

@@ -451,6 +461,10 @@ static void parse_args(int argc, char *argv[])
 case 'o':
 populate_offset = atol(optarg);
 break;
+case 'p':
+precompute_hash = true;
+hfunc = hval;
+break;

Thanks,

Emilio

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler

2018-09-11 Thread Fam Zheng

On Tue, 09/11 17:30, Paolo Bonzini wrote:
> On 11/09/2018 16:12, Fam Zheng wrote:
> > On Tue, 09/11 13:32, Paolo Bonzini wrote:
> >> On 10/09/2018 16:56, Fam Zheng wrote:
> >>> We have this unwanted call stack:
> >>>
> >>>   > ...
> >>>   > #13 0x5586602b7793 in virtio_scsi_handle_cmd_vq
> >>>   > #14 0x5586602b8d66 in virtio_scsi_data_plane_handle_cmd
> >>>   > #15 0x5586602ddab7 in virtio_queue_notify_aio_vq
> >>>   > #16 0x5586602dfc9f in virtio_queue_host_notifier_aio_poll
> >>>   > #17 0x5586607885da in run_poll_handlers_once
> >>>   > #18 0x55866078880e in try_poll_mode
> >>>   > #19 0x5586607888eb in aio_poll
> >>>   > #20 0x558660784561 in aio_wait_bh_oneshot
> >>>   > #21 0x5586602b9582 in virtio_scsi_dataplane_stop
> >>>   > #22 0x5586605a7110 in virtio_bus_stop_ioeventfd
> >>>   > #23 0x5586605a9426 in virtio_pci_stop_ioeventfd
> >>>   > #24 0x5586605ab808 in virtio_pci_common_write
> >>>   > #25 0x558660242396 in memory_region_write_accessor
> >>>   > #26 0x5586602425ab in access_with_adjusted_size
> >>>   > #27 0x558660245281 in memory_region_dispatch_write
> >>>   > #28 0x5586601e008e in flatview_write_continue
> >>>   > #29 0x5586601e01d8 in flatview_write
> >>>   > #30 0x5586601e04de in address_space_write
> >>>   > #31 0x5586601e052f in address_space_rw
> >>>   > #32 0x5586602607f2 in kvm_cpu_exec
> >>>   > #33 0x558660227148 in qemu_kvm_cpu_thread_fn
> >>>   > #34 0x55866078bde7 in qemu_thread_start
> >>>   > #35 0x7f5784906594 in start_thread
> >>>   > #36 0x7f5784639e6f in clone
> >>>
> >>> Avoid it with the aio_disable_external/aio_enable_external pair, so that
> >>> no vq poll handlers can be called in aio_wait_bh_oneshot.
> >>
> >> I don't understand.  We are in the vCPU thread, so not in the
> >> AioContext's home thread.  Why is aio_wait_bh_oneshot polling rather
> >> than going through the aio_wait_bh path?
> > 
> > What do you mean by 'aio_wait_bh path'? Here is aio_wait_bh_oneshot:
> 
> Sorry, I meant the "atomic_inc(&wait_->num_waiters);" path.  But if this
> backtrace is obtained without dataplane, that's the answer I was seeking.
> 
> > void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> > {
> > AioWaitBHData data = {
> > .cb = cb,
> > .opaque = opaque,
> > };
> > 
> > assert(qemu_get_current_aio_context() == qemu_get_aio_context());
> > 
> > aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
> > AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
> > }
> > 
> > ctx is qemu_aio_context here, so there's no interaction with IOThread.
> 
> In this case, it should be okay to have the reentrancy, what is the bug
> that this patch is fixing?

The same symptom as in the previous patch: virtio_scsi_handle_cmd_vq hangs. The
reason it hangs is fixed by the previous patch, but I don't think it should be
invoked as we're in the middle of virtio_scsi_dataplane_stop(). Applying either
one of the two patches avoids the problem, but this one is more superficial.
What do you think?

Fam

[Qemu-devel] What kind of situation to use internal or external snapshot?

2018-09-11 Thread lampahome

as title, I know there're two snapshots in qemu, but I don't know when to
use them well.

What I know is internal snapshot will save the info of L1 and L2 table in
the end of image but external snapshot won't because external snapshot
create new file to save cow data.

So when to use internal snapshot or external snapshot?

thanks to everyone.

Re: [Qemu-devel] [PATCH V1 RESEND 6/6] hmat acpi: Implement _HMA method to update HMAT at runtime

2018-09-11 Thread Liu, Jingqi



On Monday, July 16, 2018 8:29 PM, Igor Mammedov  wrote:
> On Tue, 19 Jun 2018 23:20:57 +0800
> Liu Jingqi  wrote:
> 
> > OSPM evaluates HMAT only during system initialization.
> > Any changes to the HMAT state at runtime or information regarding HMAT
> > for hot plug are communicated using _HMA method.
> >
> > _HMA is an optional object that enables the platform to provide the OS
> > with updated Heterogeneous Memory Attributes information at runtime.
> > _HMA provides OSPM with the latest HMAT in entirety overriding
> > existing HMAT.
> 
> this patch is too big and lacks any documentation how this thing is supposed 
> to
> work.
> Pls restructure and split in mode sensible chunks.
> 
> Now beside above ranting I noticed that it's build using NFIT as template.
> However it's adding extra ABI and a lot of complex code on both qemu/AML
> sides to transfer updated HMAT table to guest similar to NFIT.
> 
> I don't think that duplicating NFIT approach for every new table is 
> sustainable
> both in terms of consuming limited IO/memory resources and maintainability
> (too much complex code duplication and extra ABI to keep stable).
> 
> We should generalize/reuse NFIT code and ABI (io/memory buffer) that
> intersects with this series first and then build _HMA update on top of it.
> 
Hi Igor, 
Thanks for your all review.
We will restructure and improve the implementation.
Sorry for so late response since this development plan was postponed due to 
some urgent project.

Jingqi
> 
> > Signed-off-by: Liu Jingqi 
> > ---
> >  hw/acpi/hmat.c   | 356
> +++
> >  hw/acpi/hmat.h   |  71 ++
> >  hw/i386/acpi-build.c |   2 +
> >  hw/i386/pc.c |   2 +
> >  hw/i386/pc_piix.c|   3 +
> >  hw/i386/pc_q35.c |   3 +
> >  include/hw/i386/pc.h |   2 +
> >  7 files changed, 439 insertions(+)
> >
> > diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c index 9d29ef7..cf17c0a
> > 100644
> > --- a/hw/acpi/hmat.c
> > +++ b/hw/acpi/hmat.c
> > @@ -275,6 +275,267 @@ static void hmat_build_hma(GArray *hma,
> PCMachineState *pcms)
> >  hmat_build_cache(hma);
> >  }
> >
> > +static uint64_t
> > +hmat_hma_method_read(void *opaque, hwaddr addr, unsigned size) {
> > +printf("BUG: we never read _HMA IO Port.\n");
> > +return 0;
> > +}
> > +
> > +/* _HMA Method: read HMA data. */
> > +static void hmat_handle_hma_method(AcpiHmaState *state,
> > +   HmatHmamIn *in, hwaddr
> > +hmam_mem_addr) {
> > +HmatHmaBuffer *hma_buf = &state->hma_buf;
> > +HmatHmamOut *read_hma_out;
> > +GArray *hma;
> > +uint32_t read_len = 0, ret_status;
> > +int size;
> > +
> > +le32_to_cpus(&in->offset);
> > +
> > +hma = hma_buf->hma;
> > +if (in->offset > hma->len) {
> > +ret_status = HMAM_RET_STATUS_INVALID;
> > +goto exit;
> > +}
> > +
> > +   /* It is the first time to read HMA. */
> > +if (!in->offset) {
> > +hma_buf->dirty = false;
> > +} else if (hma_buf->dirty) { /* HMA has been changed during Reading 
> > HMA.
> */
> > +ret_status = HMAM_RET_STATUS_HMA_CHANGED;
> > +goto exit;
> > +}
> > +
> > +ret_status = HMAM_RET_STATUS_SUCCESS;
> > +read_len = MIN(hma->len - in->offset,
> > +   HMAM_MEMORY_SIZE - 2 * sizeof(uint32_t));
> > +exit:
> > +size = sizeof(HmatHmamOut) + read_len;
> > +read_hma_out = g_malloc(size);
> > +
> > +read_hma_out->len = cpu_to_le32(size);
> > +read_hma_out->ret_status = cpu_to_le32(ret_status);
> > +memcpy(read_hma_out->data, hma->data + in->offset, read_len);
> > +
> > +cpu_physical_memory_write(hmam_mem_addr, read_hma_out, size);
> > +
> > +g_free(read_hma_out);
> > +}
> > +
> > +static void
> > +hmat_hma_method_write(void *opaque, hwaddr addr, uint64_t val,
> > +unsigned size) {
> > +AcpiHmaState *state = opaque;
> > +hwaddr hmam_mem_addr = val;
> > +HmatHmamIn *in;
> > +
> > +in = g_new(HmatHmamIn, 1);
> > +cpu_physical_memory_read(hmam_mem_addr, in, sizeof(*in));
> > +
> > +hmat_handle_hma_method(state, in, hmam_mem_addr); }
> > +
> > +static const MemoryRegionOps hmat_hma_method_ops = {
> > +.read = hmat_hma_method_read,
> > +.write = hmat_hma_method_write,
> > +.endianness = DEVICE_LITTLE_ENDIAN,
> > +.valid = {
> > +.min_access_size = 4,
> > +.max_access_size = 4,
> > +},
> > +};
> > +
> > +static void hmat_init_hma_buffer(HmatHmaBuffer *hma_buf) {
> > +hma_buf->hma = g_array_new(false, true /* clear */, 1); }
> > +
> > +static uint8_t hmat_acpi_table_checksum(uint8_t *buffer, uint32_t
> > +length) {
> > +uint8_t sum = 0;
> > +uint8_t *end = buffer + length;
> > +
> > +while (buffer < end) {
> > +sum = (uint8_t) (sum + *(buffer++));
> > +}
> > +return (uint8_t)(0 - sum);
> > +}
> > +
> > +static void hmat_build_header(AcpiTableHeader *h,
> > + const char *sig, int len, uint8_t rev

Re: [Qemu-devel] [PATCH v2 12/12] qht-bench: add -p flag to precompute hash values

On 09/10/2018 11:58 AM, Emilio G. Cota wrote:
> @@ -289,7 +297,9 @@ static void htable_init(void)
>  /* avoid allocating memory later by allocating all the keys now */
>  keys = g_malloc(sizeof(*keys) * n);
>  for (i = 0; i < n; i++) {
> -keys[i] = populate_offset + i;
> +long val = populate_offset + i;
> +
> +keys[i] = precompute_hash ? h(val) : hval(val);

hfunc?


r~

[Qemu-devel] [PATCH v4 4/4] block/rbd: add deprecation documentation for filename keyvalue pairs

Signed-off-by: Jeff Cody 
---
 qemu-deprecated.texi | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 1b9c007f12..8d285b281e 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -190,6 +190,21 @@ used instead.
 In order to prevent QEMU from automatically opening an image's backing
 chain, use ``"backing": null'' instead.
 
+@subsubsection rbd keyvalue pair encoded filenames: "" (since 3.1.0)
+
+Options for ``rbd'' should be specified according to its runtime options,
+like other block drivers.  Legacy parsing of keyvalue pair encoded
+filenames is useful to open images with the old format for backing files;
+These image files should be updated to use the current format.
+
+Example of legacy encoding:
+
+@code{json:@{"file.driver":"rbd", "file.filename":"rbd:rbd/name"@}}
+
+The above, converted to the current supported format:
+
+@code{json:@{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"@}}
+
 @subsection vio-spapr-device device options
 
 @subsubsection "irq": "" (since 3.0.0)
-- 
2.17.1

[Qemu-devel] [PATCH v4 1/4] block/rbd: pull out qemu_rbd_convert_options

Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
into a helper function.

Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ca8e5bbace..b199450f9f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -655,12 +655,34 @@ failed_opts:
 return r;
 }
 
+static int qemu_rbd_convert_options(QDict *options, BlockdevOptionsRbd **opts,
+Error **errp)
+{
+Visitor *v;
+Error *local_err = NULL;
+
+/* Convert the remaining options into a QAPI object */
+v = qobject_input_visitor_new_flat_confused(options, errp);
+if (!v) {
+return -EINVAL;
+}
+
+visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
+visit_free(v);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return -EINVAL;
+}
+
+return 0;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
 BDRVRBDState *s = bs->opaque;
 BlockdevOptionsRbd *opts = NULL;
-Visitor *v;
 const QDictEntry *e;
 Error *local_err = NULL;
 char *keypairs, *secretid;
@@ -676,19 +698,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 qdict_del(options, "password-secret");
 }
 
-/* Convert the remaining options into a QAPI object */
-v = qobject_input_visitor_new_flat_confused(options, errp);
-if (!v) {
-r = -EINVAL;
-goto out;
-}
-
-visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
-visit_free(v);
-
+r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-r = -EINVAL;
 goto out;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v4 2/4] block/rbd: Attempt to parse legacy filenames

When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.

This leaves a bit of an ugly conundrum, and a hacky solution.

If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename.  Fall back to
attempting to parse the filename, and extract the required options from
it.  If that fails, pass along the original error message.

We do not support mixed modern usage alongside legacy keyvalue pair
usage.

A deprecation warning has been added, although care should be taken
when actually deprecating since the impact is not limited to
commandline or qapi usage, but also opening existing images.

Reviewed-by: Eric Blake 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 53 +++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index b199450f9f..5090e4f662 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, 
BlockdevOptionsRbd **opts,
 return 0;
 }
 
+static int qemu_rbd_attempt_legacy_options(QDict *options,
+   BlockdevOptionsRbd **opts,
+   char **keypairs)
+{
+char *filename;
+int r;
+
+filename = g_strdup(qdict_get_try_str(options, "filename"));
+if (!filename) {
+return -EINVAL;
+}
+qdict_del(options, "filename");
+
+qemu_rbd_parse_filename(filename, options, NULL);
+
+/* keypairs freed by caller */
+*keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+if (*keypairs) {
+qdict_del(options, "=keyvalue-pairs");
+}
+
+r = qemu_rbd_convert_options(options, opts, NULL);
+
+g_free(filename);
+return r;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
-error_propagate(errp, local_err);
-goto out;
+/* If keypairs are present, that means some options are present in
+ * the modern option format.  Don't attempt to parse legacy option
+ * formats, as we won't support mixed usage. */
+if (keypairs) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+/* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename.  Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options. */
+r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
+if (r < 0) {
+error_propagate(errp, local_err);
+goto out;
+}
+/* Take care whenever deciding to actually deprecate; once this ability
+ * is removed, we will not be able to open any images with 
legacy-styled
+ * backing image strings. */
+error_report("RBD options encoded in the filename as keyvalue pairs "
+ "is deprecated.  Future versions may cease to parse "
+ "these options in the future.");
 }
 
 /* Remove the processed options from the QDict (the visitor processes
-- 
2.17.1

[Qemu-devel] [PATCH v4 0/4] block/rbd: enable filename parsing on open

Changes from v3:


Patch 4: Typo fixed [Eric]
 Added examples [Eric]

Changes from v2:
=

Patch 4: New, document deprecation. [Eric]
Patch 3,2: Add r-b's


Changes from v1:
=

Patch 1: Don't pass unused BlockDriverState to helper function

Patch 2: Do not allow mixed usage; fail if keyvalue is present [Eric]
 Add deprecation warning [John]
 Pull legacy parsing code into function [John]
 Fixed filename leak

Patch 3: New; iotest 231. [Eric]


iotest failure on current master:

 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
-unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
-qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
Parameter 'pool' is missing
 unable to get monitor info from DNS SRV with service name: ceph-mon
 no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
Failures: 231
Failed 1 of 1 tests

Jeff Cody (4):
  block/rbd: pull out qemu_rbd_convert_options
  block/rbd: Attempt to parse legacy filenames
  block/rbd: add iotest for rbd legacy keyvalue filename parsing
  block/rbd: add deprecation documentation for filename keyvalue pairs

 block/rbd.c| 89 --
 qemu-deprecated.texi   | 15 +++
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 
 tests/qemu-iotests/group   |  1 +
 5 files changed, 162 insertions(+), 14 deletions(-)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

-- 
2.17.1

[Qemu-devel] [PATCH v4 3/4] block/rbd: add iotest for rbd legacy keyvalue filename parsing

This is a small test that will check for the ability to parse
both legacy and modern options for rbd.

The way the test is set up is for failure to occur, but without
having to wait to timeout on a non-existent rbd server.  The error
messages in the success path show that the arguments were parsed.

The failure behavior prior to the patch series that has this test, is
qemu-img complaining about mandatory options (e.g. 'pool') not being
provided.

Reviewed-by: Eric Blake 
Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 72 insertions(+)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
new file mode 100755
index 00..3e283708b4
--- /dev/null
+++ b/tests/qemu-iotests/231
@@ -0,0 +1,62 @@
+#!/bin/bash
+#
+# Test legacy and modern option parsing for rbd/ceph.  This will not
+# actually connect to a ceph server, but rather looks for the appropriate
+# error message that indicates we parsed the options correctly.
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+_cleanup()
+{
+rm "${BOGUS_CONF}"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto rbd
+_supported_os Linux
+
+BOGUS_CONF=${TEST_DIR}/ceph-$$.conf
+touch "${BOGUS_CONF}"
+
+_filter_conf()
+{
+sed -e "s#$BOGUS_CONF#BOGUS_CONF#g"
+}
+
+# We expect this to fail, with no monitor ip provided and a null conf file.  
Just want it
+# to fail in the right way.
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
new file mode 100644
index 00..579ba11c16
--- /dev/null
+++ b/tests/qemu-iotests/231.out
@@ -0,0 +1,9 @@
+QA output created by 231
+qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 743790745b..31f6e77dcb 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -226,3 +226,4 @@
 226 auto quick
 227 auto quick
 229 auto quick
+231 auto quick
-- 
2.17.1

Re: [Qemu-devel] [PATCH v3 4/4] block/rbd: add deprecation documenation for filename keyvalue pairs

On Tue, Sep 11, 2018 at 04:56:36PM -0500, Eric Blake wrote:
> [MAINTAINERS says libvir-list should have been cc'd; not sure why that
> didn't happen]
> 

Thanks

> On 9/11/18 4:34 PM, Jeff Cody wrote:
> >Signed-off-by: Jeff Cody 
> 
> In the subject: s/documenation/documentation/
> 
> >---
> >  qemu-deprecated.texi | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> >diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> >index 1b9c007f12..4df8ac442d 100644
> >--- a/qemu-deprecated.texi
> >+++ b/qemu-deprecated.texi
> >@@ -190,6 +190,13 @@ used instead.
> >  In order to prevent QEMU from automatically opening an image's backing
> >  chain, use ``"backing": null'' instead.
> >+@subsubsection "rbd keyvalue pair encoded filenames": "" (since 3.1.0)
> >+
> >+Options for ``rbd'' should be specified according to its runtime options,
> >+like other block drivers.  Legacy parsing of keyvalue pair encoded
> >+filenames is useful to open images with the old format for backing files;
> >+These image files should be updated to use the current format.
> 
> Can we give an example?  Cribbing from patch 3, an example might look like
> changing:
> 
> json:{"file.driver":"rbd", "file.filename":"rbd:rbd/name"}
> 
> into:
> 
> json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
> 

That is a good example, I'll include it.

> I'll let Peter or John comment on whether libvirt's RBD pool handler is
> impacted by this deprecation, but it seems reasonable to me.
> 

Thanks!

Re: [Qemu-devel] [PATCH] PC Chipset: Improve serial divisor calculation

2018-09-11 Thread Guenter Roeck

On Tue, Sep 11, 2018 at 03:21:34PM +0200, Paolo Bonzini wrote:
> On 01/09/2018 01:37, Guenter Roeck wrote:
> > The patch results in an unexpected DLL register value. Here is the
> > surrounding code from drivers/tty/serial/pxa.c:
> > 
> > serial_out(up, UART_DLL, quot & 0xff);  /* LS of divisor */
> > 
> > /*
> >  * work around Errata #75 according to Intel(R) PXA27x
> >  * Processor Family Specification Update (Nov 2005)
> >  */
> > dll = serial_in(up, UART_DLL);
> > WARN_ON(dll != (quot & 0xff));  // <-- warning
> > 
> > Reverting the patch fixes the problem.
> 
> I think it's a typo.  Can you try this:
> 

You are correct, the patch below fixes the problem.

Tested-by: Guenter Roeck 

Thanks!
Guenter

> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 251f40fdac..02463e3388 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -345,9 +345,9 @@ static void serial_ioport_write(void *opaque, hwaddr 
> addr, uint64_t val,
>  default:
>  case 0:
>  if (s->lcr & UART_LCR_DLAB) {
> -if (size == 2) {
> +if (size == 1) {
>  s->divider = (s->divider & 0xff00) | val;
> -} else if (size == 4) {
> +} else {
>  s->divider = val;
>  }
>  serial_update_parameters(s);
> 
> Thanks,
> 
> Paolo

Re: [Qemu-devel] [PATCH v3 4/4] block/rbd: add deprecation documenation for filename keyvalue pairs

[MAINTAINERS says libvir-list should have been cc'd; not sure why that 
didn't happen]


On 9/11/18 4:34 PM, Jeff Cody wrote:

Signed-off-by: Jeff Cody 


In the subject: s/documenation/documentation/


---
  qemu-deprecated.texi | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 1b9c007f12..4df8ac442d 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -190,6 +190,13 @@ used instead.
  In order to prevent QEMU from automatically opening an image's backing
  chain, use ``"backing": null'' instead.
  
+@subsubsection "rbd keyvalue pair encoded filenames": "" (since 3.1.0)

+
+Options for ``rbd'' should be specified according to its runtime options,
+like other block drivers.  Legacy parsing of keyvalue pair encoded
+filenames is useful to open images with the old format for backing files;
+These image files should be updated to use the current format.


Can we give an example?  Cribbing from patch 3, an example might look 
like changing:


json:{"file.driver":"rbd", "file.filename":"rbd:rbd/name"}

into:

json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}

I'll let Peter or John comment on whether libvirt's RBD pool handler is 
impacted by this deprecation, but it seems reasonable to me.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH v3 2/4] block/rbd: Attempt to parse legacy filenames

When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.

This leaves a bit of an ugly conundrum, and a hacky solution.

If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename.  Fall back to
attempting to parse the filename, and extract the required options from
it.  If that fails, pass along the original error message.

We do not support mixed modern usage alongside legacy keyvalue pair
usage.

A deprecation warning has been added, although care should be taken
when actually deprecating since the impact is not limited to
commandline or qapi usage, but also opening existing images.

Reviewed-by: Eric Blake 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 53 +++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index b199450f9f..5090e4f662 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, 
BlockdevOptionsRbd **opts,
 return 0;
 }
 
+static int qemu_rbd_attempt_legacy_options(QDict *options,
+   BlockdevOptionsRbd **opts,
+   char **keypairs)
+{
+char *filename;
+int r;
+
+filename = g_strdup(qdict_get_try_str(options, "filename"));
+if (!filename) {
+return -EINVAL;
+}
+qdict_del(options, "filename");
+
+qemu_rbd_parse_filename(filename, options, NULL);
+
+/* keypairs freed by caller */
+*keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+if (*keypairs) {
+qdict_del(options, "=keyvalue-pairs");
+}
+
+r = qemu_rbd_convert_options(options, opts, NULL);
+
+g_free(filename);
+return r;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
-error_propagate(errp, local_err);
-goto out;
+/* If keypairs are present, that means some options are present in
+ * the modern option format.  Don't attempt to parse legacy option
+ * formats, as we won't support mixed usage. */
+if (keypairs) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+/* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename.  Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options. */
+r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
+if (r < 0) {
+error_propagate(errp, local_err);
+goto out;
+}
+/* Take care whenever deciding to actually deprecate; once this ability
+ * is removed, we will not be able to open any images with 
legacy-styled
+ * backing image strings. */
+error_report("RBD options encoded in the filename as keyvalue pairs "
+ "is deprecated.  Future versions may cease to parse "
+ "these options in the future.");
 }
 
 /* Remove the processed options from the QDict (the visitor processes
-- 
2.17.1

[Qemu-devel] [PATCH v3 1/4] block/rbd: pull out qemu_rbd_convert_options

Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
into a helper function.

Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ca8e5bbace..b199450f9f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -655,12 +655,34 @@ failed_opts:
 return r;
 }
 
+static int qemu_rbd_convert_options(QDict *options, BlockdevOptionsRbd **opts,
+Error **errp)
+{
+Visitor *v;
+Error *local_err = NULL;
+
+/* Convert the remaining options into a QAPI object */
+v = qobject_input_visitor_new_flat_confused(options, errp);
+if (!v) {
+return -EINVAL;
+}
+
+visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
+visit_free(v);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return -EINVAL;
+}
+
+return 0;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
 BDRVRBDState *s = bs->opaque;
 BlockdevOptionsRbd *opts = NULL;
-Visitor *v;
 const QDictEntry *e;
 Error *local_err = NULL;
 char *keypairs, *secretid;
@@ -676,19 +698,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 qdict_del(options, "password-secret");
 }
 
-/* Convert the remaining options into a QAPI object */
-v = qobject_input_visitor_new_flat_confused(options, errp);
-if (!v) {
-r = -EINVAL;
-goto out;
-}
-
-visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
-visit_free(v);
-
+r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-r = -EINVAL;
 goto out;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH v3 0/4] block/rbd: enable filename parsing on open

Changes from v2:
=

Patch 4: New, document deprecation. [Eric]
Patch 3,2: Add r-b's


Changes from v1:
=

Patch 1: Don't pass unused BlockDriverState to helper function

Patch 2: Do not allow mixed usage; fail if keyvalue is present [Eric]
 Add deprecation warning [John]
 Pull legacy parsing code into function [John]
 Fixed filename leak

Patch 3: New; iotest 231. [Eric]


iotest failure on current master:

 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
-unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
-qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
Parameter 'pool' is missing
 unable to get monitor info from DNS SRV with service name: ceph-mon
 no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
Failures: 231
Failed 1 of 1 tests

Jeff Cody (4):
  block/rbd: pull out qemu_rbd_convert_options
  block/rbd: Attempt to parse legacy filenames
  block/rbd: add iotest for rbd legacy keyvalue filename parsing
  block/rbd: add deprecation documenation for filename keyvalue pairs

 block/rbd.c| 89 --
 qemu-deprecated.texi   |  7 +++
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 
 tests/qemu-iotests/group   |  1 +
 5 files changed, 154 insertions(+), 14 deletions(-)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

-- 
2.17.1

[Qemu-devel] [PATCH v3 3/4] block/rbd: add iotest for rbd legacy keyvalue filename parsing

This is a small test that will check for the ability to parse
both legacy and modern options for rbd.

The way the test is set up is for failure to occur, but without
having to wait to timeout on a non-existent rbd server.  The error
messages in the success path show that the arguments were parsed.

The failure behavior prior to the patch series that has this test, is
qemu-img complaining about mandatory options (e.g. 'pool') not being
provided.

Reviewed-by: Eric Blake 
Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 72 insertions(+)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
new file mode 100755
index 00..3e283708b4
--- /dev/null
+++ b/tests/qemu-iotests/231
@@ -0,0 +1,62 @@
+#!/bin/bash
+#
+# Test legacy and modern option parsing for rbd/ceph.  This will not
+# actually connect to a ceph server, but rather looks for the appropriate
+# error message that indicates we parsed the options correctly.
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+_cleanup()
+{
+rm "${BOGUS_CONF}"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto rbd
+_supported_os Linux
+
+BOGUS_CONF=${TEST_DIR}/ceph-$$.conf
+touch "${BOGUS_CONF}"
+
+_filter_conf()
+{
+sed -e "s#$BOGUS_CONF#BOGUS_CONF#g"
+}
+
+# We expect this to fail, with no monitor ip provided and a null conf file.  
Just want it
+# to fail in the right way.
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
new file mode 100644
index 00..579ba11c16
--- /dev/null
+++ b/tests/qemu-iotests/231.out
@@ -0,0 +1,9 @@
+QA output created by 231
+qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 743790745b..31f6e77dcb 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -226,3 +226,4 @@
 226 auto quick
 227 auto quick
 229 auto quick
+231 auto quick
-- 
2.17.1

[Qemu-devel] [PATCH v3 4/4] block/rbd: add deprecation documenation for filename keyvalue pairs

Signed-off-by: Jeff Cody 
---
 qemu-deprecated.texi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 1b9c007f12..4df8ac442d 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -190,6 +190,13 @@ used instead.
 In order to prevent QEMU from automatically opening an image's backing
 chain, use ``"backing": null'' instead.
 
+@subsubsection "rbd keyvalue pair encoded filenames": "" (since 3.1.0)
+
+Options for ``rbd'' should be specified according to its runtime options,
+like other block drivers.  Legacy parsing of keyvalue pair encoded
+filenames is useful to open images with the old format for backing files;
+These image files should be updated to use the current format.
+
 @subsection vio-spapr-device device options
 
 @subsubsection "irq": "" (since 3.0.0)
-- 
2.17.1

[Qemu-devel] [PATCH] Fix breakpoints in nios2 user-mode emulation.

2018-09-11 Thread Sandra Loosemore

Without this patch, QEMU exits immediately when it execution stops at
a breakpoint, instead of reporting it to GDB.

Signed-off-by: Sandra Loosemore 
---
 linux-user/nios2/cpu_loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/nios2/cpu_loop.c b/linux-user/nios2/cpu_loop.c
index dac7a06..a5ae37f 100644
--- a/linux-user/nios2/cpu_loop.c
+++ b/linux-user/nios2/cpu_loop.c
@@ -71,6 +71,9 @@ void cpu_loop(CPUNios2State *env)
 gdbsig = TARGET_SIGTRAP;
 break;
 }
+case EXCP_DEBUG:
+gdbsig = TARGET_SIGTRAP;
+break;
 case 0xaa:
 switch (env->regs[R_PC]) {
 /*case 0x1000:*/  /* TODO:__kuser_helper_version */
-- 
2.8.1

Re: [Qemu-devel] [PATCH v2 2/3] block/rbd: Attempt to parse legacy filenames

On Tue, Sep 11, 2018 at 04:05:45PM -0500, Eric Blake wrote:
> On 9/11/18 3:43 PM, Jeff Cody wrote:
> >When we converted rbd to get rid of the older key/value-centric
> >encoding format, we broke compatibility with image files with backing
> >file strings encoded in the old format.
> >
> >This leaves a bit of an ugly conundrum, and a hacky solution.
> >
> >If the initial attempt to parse the "proper" options fails, it assumes
> >that we may have an older key/value encoded filename.  Fall back to
> >attempting to parse the filename, and extract the required options from
> >it.  If that fails, pass along the original error message.
> >
> >We do not support mixed modern usage alongside legacy keyvalue pair
> >usage.
> >
> >A deprecation warning has been added, although care should be taken
> >when actually deprecating since the impact is not limited to
> >commandline or qapi usage, but also opening existing images.
> >
> >Signed-off-by: Jeff Cody 
> >---
> >  block/rbd.c | 53 +++--
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> Where's the patch to qemu/qemu-deprecated.texi to mention the new message
> and our advice on upgrading images to avoid triggering it?
> 

Argh, thanks - forgot that.  I'll spin a v3 with that as patch 4.

> >
> >diff --git a/block/rbd.c b/block/rbd.c
> >index b199450f9f..5090e4f662 100644
> >--- a/block/rbd.c
> >+++ b/block/rbd.c
> >@@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, 
> >BlockdevOptionsRbd **opts,
> >  return 0;
> >  }
> >+static int qemu_rbd_attempt_legacy_options(QDict *options,
> >+   BlockdevOptionsRbd **opts,
> >+   char **keypairs)
> >+{
> >+char *filename;
> >+int r;
> >+
> >+filename = g_strdup(qdict_get_try_str(options, "filename"));
> >+if (!filename) {
> >+return -EINVAL;
> >+}
> >+qdict_del(options, "filename");
> >+
> >+qemu_rbd_parse_filename(filename, options, NULL);
> 
> Intentionally ignoring errors here,
> 
> >+
> >+/* keypairs freed by caller */
> >+*keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> >+if (*keypairs) {
> >+qdict_del(options, "=keyvalue-pairs");
> >+}
> >+
> >+r = qemu_rbd_convert_options(options, opts, NULL);
> 
> and here. I guess we'll see how the caller is expecting things to behave.
> 
> >+
> >+g_free(filename);
> >+return r;
> >+}
> >+
> >  static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
> >   Error **errp)
> >  {
> >@@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> >*options, int flags,
> >  r = qemu_rbd_convert_options(options, &opts, &local_err);
> >  if (local_err) {
> >-error_propagate(errp, local_err);
> >-goto out;
> >+/* If keypairs are present, that means some options are present in
> >+ * the modern option format.  Don't attempt to parse legacy option
> >+ * formats, as we won't support mixed usage. */
> >+if (keypairs) {
> >+error_propagate(errp, local_err);
> >+goto out;
> >+}
> >+
> >+/* If the initial attempt to convert and process the options failed,
> >+ * we may be attempting to open an image file that has the rbd 
> >options
> >+ * specified in the older format consisting of all key/value pairs
> >+ * encoded in the filename.  Go ahead and attempt to parse the
> >+ * filename, and see if we can pull out the required options. */
> >+r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
> >+if (r < 0) {
> >+error_propagate(errp, local_err);
> >+goto out;
> >+}
> >+/* Take care whenever deciding to actually deprecate; once this 
> >ability
> >+ * is removed, we will not be able to open any images with 
> >legacy-styled
> >+ * backing image strings. */
> >+error_report("RBD options encoded in the filename as keyvalue pairs 
> >"
> >+ "is deprecated.  Future versions may cease to parse "
> >+ "these options in the future.");
> 
> Okay, so you're making a best-effort fallback to scrape out any remaining
> keyvalue pairs. If there was no filename (and hence no keyvalue pairs
> populated), we know up front that we lack enough information, so
> attempt_legacy_options() returns failure right away; if there was a
> filename, we don't know if we got all the required options (so ignoring
> errors was okay) - that will be up to later code to decipher, after we emit
> our warning that we already relied on legacy options (if the later code has
> all it needed, then only the warning is emitted; if later code fails, the
> user now has the warning in addition to the later failure message to help
> them resolve their images).
> 
> A bit confusing how

Re: [Qemu-devel] [PATCH v2 3/3] block/rbd: add iotest for rbd legacy keyvalue filename parsing


On 9/11/18 3:43 PM, Jeff Cody wrote:

This is a small test that will check for the ability to parse
both legacy and modern options for rbd.

The way the test is set up is for failure to occur, but without
having to wait to timeout on a non-existent rbd server.  The error
messages in the success path show that the arguments were parsed.

The failure behavior prior to the patch series that has this test, is
qemu-img complaining about mandatory options (e.g. 'pool') not being
provided.

Signed-off-by: Jeff Cody 
---
  tests/qemu-iotests/231 | 62 ++
  tests/qemu-iotests/231.out |  9 ++
  tests/qemu-iotests/group   |  1 +
  3 files changed, 72 insertions(+)
  create mode 100755 tests/qemu-iotests/231
  create mode 100644 tests/qemu-iotests/231.out


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 2/3] block/rbd: Attempt to parse legacy filenames


On 9/11/18 3:43 PM, Jeff Cody wrote:

When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.

This leaves a bit of an ugly conundrum, and a hacky solution.

If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename.  Fall back to
attempting to parse the filename, and extract the required options from
it.  If that fails, pass along the original error message.

We do not support mixed modern usage alongside legacy keyvalue pair
usage.

A deprecation warning has been added, although care should be taken
when actually deprecating since the impact is not limited to
commandline or qapi usage, but also opening existing images.

Signed-off-by: Jeff Cody 
---
  block/rbd.c | 53 +++--
  1 file changed, 51 insertions(+), 2 deletions(-)


Where's the patch to qemu/qemu-deprecated.texi to mention the new 
message and our advice on upgrading images to avoid triggering it?




diff --git a/block/rbd.c b/block/rbd.c
index b199450f9f..5090e4f662 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, 
BlockdevOptionsRbd **opts,
  return 0;
  }
  
+static int qemu_rbd_attempt_legacy_options(QDict *options,

+   BlockdevOptionsRbd **opts,
+   char **keypairs)
+{
+char *filename;
+int r;
+
+filename = g_strdup(qdict_get_try_str(options, "filename"));
+if (!filename) {
+return -EINVAL;
+}
+qdict_del(options, "filename");
+
+qemu_rbd_parse_filename(filename, options, NULL);


Intentionally ignoring errors here,


+
+/* keypairs freed by caller */
+*keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+if (*keypairs) {
+qdict_del(options, "=keyvalue-pairs");
+}
+
+r = qemu_rbd_convert_options(options, opts, NULL);


and here. I guess we'll see how the caller is expecting things to behave.


+
+g_free(filename);
+return r;
+}
+
  static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
   Error **errp)
  {
@@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
  
  r = qemu_rbd_convert_options(options, &opts, &local_err);

  if (local_err) {
-error_propagate(errp, local_err);
-goto out;
+/* If keypairs are present, that means some options are present in
+ * the modern option format.  Don't attempt to parse legacy option
+ * formats, as we won't support mixed usage. */
+if (keypairs) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+/* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename.  Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options. */
+r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
+if (r < 0) {
+error_propagate(errp, local_err);
+goto out;
+}
+/* Take care whenever deciding to actually deprecate; once this ability
+ * is removed, we will not be able to open any images with 
legacy-styled
+ * backing image strings. */
+error_report("RBD options encoded in the filename as keyvalue pairs "
+ "is deprecated.  Future versions may cease to parse "
+ "these options in the future.");


Okay, so you're making a best-effort fallback to scrape out any 
remaining keyvalue pairs. If there was no filename (and hence no 
keyvalue pairs populated), we know up front that we lack enough 
information, so attempt_legacy_options() returns failure right away; if 
there was a filename, we don't know if we got all the required options 
(so ignoring errors was okay) - that will be up to later code to 
decipher, after we emit our warning that we already relied on legacy 
options (if the later code has all it needed, then only the warning is 
emitted; if later code fails, the user now has the warning in addition 
to the later failure message to help them resolve their images).


A bit confusing how it all fits together, but it seems to work out in 
the end.


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 11/13] target/i386: move cpu_tmp1_i64 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 160 
>  1 file changed, 80 insertions(+), 80 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 12/13] target/i386: move x86_64_hregs to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> And convert it to a bool to use an existing hole
> in the struct.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 307 
>  1 file changed, 154 insertions(+), 153 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 10/13] target/i386: move cpu_tmp3_i32 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 64 -
>  1 file changed, 32 insertions(+), 32 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 09/13] target/i386: move cpu_tmp2_i32 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 347 
>  1 file changed, 174 insertions(+), 173 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 07/13] target/i386: move cpu_ptr0 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 101 +---
>  1 file changed, 52 insertions(+), 49 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 08/13] target/i386: move cpu_ptr1 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 52 -
>  1 file changed, 26 insertions(+), 26 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 06/13] target/i386: move cpu_tmp4 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 78 -
>  1 file changed, 39 insertions(+), 39 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 05/13] target/i386: move cpu_tmp0 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 282 
>  1 file changed, 144 insertions(+), 138 deletions(-)

Reviewed-by: Richard Henderson 

I will note that these tmpN variables ought to be eliminated
completely, in favor of totally local temporary allocation.

But that should be done separately, because while they *ought*
to be local, in some cases it may be hard to see that they are.

r~

[Qemu-devel] [PATCH v2 0/3] block/rbd: enable filename parsing on open

Changes from v1:

Patch 1: Don't pass unused BlockDriverState to helper function

Patch 2: Do not allow mixed usage; fail if keyvalue is present [Eric]
 Add deprecation warning [John]
 Pull legacy parsing code into function [John]
 Fixed filename leak

Patch 3: New; iotest 231. [Eric]


iotest failure on current master:

 QA output created by 231
-qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
-unable to get monitor info from DNS SRV with service name: ceph-mon
-no monitors specified to connect to.
-qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
Parameter 'pool' is missing
 unable to get monitor info from DNS SRV with service name: ceph-mon
 no monitors specified to connect to.
 qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
Failures: 231
Failed 1 of 1 tests



Jeff Cody (3):
  block/rbd: pull out qemu_rbd_convert_options
  block/rbd: Attempt to parse legacy filenames
  block/rbd: add iotest for rbd legacy keyvalue filename parsing

 block/rbd.c| 89 --
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 
 tests/qemu-iotests/group   |  1 +
 4 files changed, 147 insertions(+), 14 deletions(-)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

-- 
2.17.1

Re: [Qemu-devel] [PATCH v3 03/13] target/i386: move cpu_T0 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 1174 ---
>  1 file changed, 594 insertions(+), 580 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 02/13] target/i386: move cpu_A0 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 472 
>  1 file changed, 236 insertions(+), 236 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 04/13] target/i386: move cpu_T1 to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 341 
>  1 file changed, 170 insertions(+), 171 deletions(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v2 3/3] block/rbd: add iotest for rbd legacy keyvalue filename parsing

This is a small test that will check for the ability to parse
both legacy and modern options for rbd.

The way the test is set up is for failure to occur, but without
having to wait to timeout on a non-existent rbd server.  The error
messages in the success path show that the arguments were parsed.

The failure behavior prior to the patch series that has this test, is
qemu-img complaining about mandatory options (e.g. 'pool') not being
provided.

Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/231 | 62 ++
 tests/qemu-iotests/231.out |  9 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 72 insertions(+)
 create mode 100755 tests/qemu-iotests/231
 create mode 100644 tests/qemu-iotests/231.out

diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231
new file mode 100755
index 00..3e283708b4
--- /dev/null
+++ b/tests/qemu-iotests/231
@@ -0,0 +1,62 @@
+#!/bin/bash
+#
+# Test legacy and modern option parsing for rbd/ceph.  This will not
+# actually connect to a ceph server, but rather looks for the appropriate
+# error message that indicates we parsed the options correctly.
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+_cleanup()
+{
+rm "${BOGUS_CONF}"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto rbd
+_supported_os Linux
+
+BOGUS_CONF=${TEST_DIR}/ceph-$$.conf
+touch "${BOGUS_CONF}"
+
+_filter_conf()
+{
+sed -e "s#$BOGUS_CONF#BOGUS_CONF#g"
+}
+
+# We expect this to fail, with no monitor ip provided and a null conf file.  
Just want it
+# to fail in the right way.
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 
2>&1 | _filter_conf
+$QEMU_IMG info 
"json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}"
 2>&1 | _filter_conf
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out
new file mode 100644
index 00..579ba11c16
--- /dev/null
+++ b/tests/qemu-iotests/231.out
@@ -0,0 +1,9 @@
+QA output created by 231
+qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. 
 Future versions may cease to parse these options in the future.
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': 
error connecting: No such file or directory
+unable to get monitor info from DNS SRV with service name: ceph-mon
+no monitors specified to connect to.
+qemu-img: Could not open 
'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}':
 error connecting: No such file or directory
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 743790745b..31f6e77dcb 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -226,3 +226,4 @@
 226 auto quick
 227 auto quick
 229 auto quick
+231 auto quick
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/3] block/rbd: Attempt to parse legacy filenames

When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.

This leaves a bit of an ugly conundrum, and a hacky solution.

If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename.  Fall back to
attempting to parse the filename, and extract the required options from
it.  If that fails, pass along the original error message.

We do not support mixed modern usage alongside legacy keyvalue pair
usage.

A deprecation warning has been added, although care should be taken
when actually deprecating since the impact is not limited to
commandline or qapi usage, but also opening existing images.

Signed-off-by: Jeff Cody 
---
 block/rbd.c | 53 +++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index b199450f9f..5090e4f662 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, 
BlockdevOptionsRbd **opts,
 return 0;
 }
 
+static int qemu_rbd_attempt_legacy_options(QDict *options,
+   BlockdevOptionsRbd **opts,
+   char **keypairs)
+{
+char *filename;
+int r;
+
+filename = g_strdup(qdict_get_try_str(options, "filename"));
+if (!filename) {
+return -EINVAL;
+}
+qdict_del(options, "filename");
+
+qemu_rbd_parse_filename(filename, options, NULL);
+
+/* keypairs freed by caller */
+*keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+if (*keypairs) {
+qdict_del(options, "=keyvalue-pairs");
+}
+
+r = qemu_rbd_convert_options(options, opts, NULL);
+
+g_free(filename);
+return r;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
-error_propagate(errp, local_err);
-goto out;
+/* If keypairs are present, that means some options are present in
+ * the modern option format.  Don't attempt to parse legacy option
+ * formats, as we won't support mixed usage. */
+if (keypairs) {
+error_propagate(errp, local_err);
+goto out;
+}
+
+/* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename.  Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options. */
+r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
+if (r < 0) {
+error_propagate(errp, local_err);
+goto out;
+}
+/* Take care whenever deciding to actually deprecate; once this ability
+ * is removed, we will not be able to open any images with 
legacy-styled
+ * backing image strings. */
+error_report("RBD options encoded in the filename as keyvalue pairs "
+ "is deprecated.  Future versions may cease to parse "
+ "these options in the future.");
 }
 
 /* Remove the processed options from the QDict (the visitor processes
-- 
2.17.1

Re: [Qemu-devel] [PATCH v3 01/13] target/i386: move cpu_cc_srcT to DisasContext

On 09/11/2018 01:28 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 32 ++--
>  1 file changed, 18 insertions(+), 14 deletions(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v2 1/3] block/rbd: pull out qemu_rbd_convert_options

Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
into a helper function.

Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ca8e5bbace..b199450f9f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -655,12 +655,34 @@ failed_opts:
 return r;
 }
 
+static int qemu_rbd_convert_options(QDict *options, BlockdevOptionsRbd **opts,
+Error **errp)
+{
+Visitor *v;
+Error *local_err = NULL;
+
+/* Convert the remaining options into a QAPI object */
+v = qobject_input_visitor_new_flat_confused(options, errp);
+if (!v) {
+return -EINVAL;
+}
+
+visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
+visit_free(v);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return -EINVAL;
+}
+
+return 0;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
 BDRVRBDState *s = bs->opaque;
 BlockdevOptionsRbd *opts = NULL;
-Visitor *v;
 const QDictEntry *e;
 Error *local_err = NULL;
 char *keypairs, *secretid;
@@ -676,19 +698,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 qdict_del(options, "password-secret");
 }
 
-/* Convert the remaining options into a QAPI object */
-v = qobject_input_visitor_new_flat_confused(options, errp);
-if (!v) {
-r = -EINVAL;
-goto out;
-}
-
-visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
-visit_free(v);
-
+r = qemu_rbd_convert_options(options, &opts, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-r = -EINVAL;
 goto out;
 }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

On Tue, Sep 11, 2018 at 05:43:38 -0700, Richard Henderson wrote:
> On 09/10/2018 04:27 PM, Emilio G. Cota wrote:
> > +#define GEN_READ(name, type)\
> > +type name(const type *ptr)  \
> > +{   \
> > +QemuSpin *lock = addr_to_lock(ptr); \
> > +type ret;   \
> > +\
> > +qemu_spin_lock(lock);   \
> > +ret = *ptr; \
> > +qemu_spin_unlock(lock); \
> > +return ret; \
> > +}
> > +
> > +GEN_READ(atomic_read_i64, int64_t)
> > +GEN_READ(atomic_read_u64, uint64_t)
> 
> Is there really a good reason to have two external
> functions instead of having one of them inline and
> perform a cast?

Not really. I can do a follow-up patch if you want me to.

> Is this any better than using libatomic?

I didn't think of using libatomic. I just checked the source
code and it's quite similar:
- It uses 64 locks instead of 16 ($page_size/$cache_line,
  but these are hard-coded for POSIX as 4096/64, respectively)
- We compute the cacheline size and corresponding padding
  at run-time, which is a little better than the above.
- The locks are implemented as pthread_mutex instead of
  spinlocks. I think spinlocks make more sense here because
  we do not expect huge contention (systems without
  !CONFIG_ATOMIC64 have few cores); for libatomic it makes
  sense to use mutexes because it might be used in many-core
  machines.

So yes, we could have used libatomic. If you feel strongly
about it, I can give it a shot.

Thanks,

Emilio

[Qemu-devel] [PATCH v3 09/13] target/i386: move cpu_tmp2_i32 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 347 
 1 file changed, 174 insertions(+), 173 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index c51f61ca2c..ec68f7dba1 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,7 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv_i32 cpu_tmp2_i32, cpu_tmp3_i32;
+static TCGv_i32 cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
 
 #include "exec/gen-icount.h"
@@ -142,6 +142,7 @@ typedef struct DisasContext {
 TCGv tmp4;
 TCGv_ptr ptr0;
 TCGv_ptr ptr1;
+TCGv_i32 tmp2_i32;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -617,16 +618,16 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 target_ulong next_eip;
 
 if (s->pe && (s->cpl > s->iopl || s->vm86)) {
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
+tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
 switch (ot) {
 case MO_8:
-gen_helper_check_iob(cpu_env, cpu_tmp2_i32);
+gen_helper_check_iob(cpu_env, s->tmp2_i32);
 break;
 case MO_16:
-gen_helper_check_iow(cpu_env, cpu_tmp2_i32);
+gen_helper_check_iow(cpu_env, s->tmp2_i32);
 break;
 case MO_32:
-gen_helper_check_iol(cpu_env, cpu_tmp2_i32);
+gen_helper_check_iol(cpu_env, s->tmp2_i32);
 break;
 default:
 tcg_abort();
@@ -637,8 +638,8 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 gen_jmp_im(s, cur_eip);
 svm_flags |= (1 << (4 + ot));
 next_eip = s->pc - s->cs_base;
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
-gen_helper_svm_check_io(cpu_env, cpu_tmp2_i32,
+tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
+gen_helper_svm_check_io(cpu_env, s->tmp2_i32,
 tcg_const_i32(svm_flags),
 tcg_const_i32(next_eip - cur_eip));
 }
@@ -1136,13 +1137,13 @@ static inline void gen_ins(DisasContext *s, TCGMemOp ot)
case of page fault. */
 tcg_gen_movi_tl(s->T0, 0);
 gen_op_st_v(s, ot, s->T0, s->A0);
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_regs[R_EDX]);
-tcg_gen_andi_i32(cpu_tmp2_i32, cpu_tmp2_i32, 0x);
-gen_helper_in_func(ot, s->T0, cpu_tmp2_i32);
+tcg_gen_trunc_tl_i32(s->tmp2_i32, cpu_regs[R_EDX]);
+tcg_gen_andi_i32(s->tmp2_i32, s->tmp2_i32, 0x);
+gen_helper_in_func(ot, s->T0, s->tmp2_i32);
 gen_op_st_v(s, ot, s->T0, s->A0);
 gen_op_movl_T0_Dshift(s, ot);
 gen_op_add_reg_T0(s, s->aflag, R_EDI);
-gen_bpt_io(s, cpu_tmp2_i32, ot);
+gen_bpt_io(s, s->tmp2_i32, ot);
 if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
 gen_io_end();
 }
@@ -1156,13 +1157,13 @@ static inline void gen_outs(DisasContext *s, TCGMemOp 
ot)
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, s->T0, s->A0);
 
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_regs[R_EDX]);
-tcg_gen_andi_i32(cpu_tmp2_i32, cpu_tmp2_i32, 0x);
+tcg_gen_trunc_tl_i32(s->tmp2_i32, cpu_regs[R_EDX]);
+tcg_gen_andi_i32(s->tmp2_i32, s->tmp2_i32, 0x);
 tcg_gen_trunc_tl_i32(cpu_tmp3_i32, s->T0);
-gen_helper_out_func(ot, cpu_tmp2_i32, cpu_tmp3_i32);
+gen_helper_out_func(ot, s->tmp2_i32, cpu_tmp3_i32);
 gen_op_movl_T0_Dshift(s, ot);
 gen_op_add_reg_T0(s, s->aflag, R_ESI);
-gen_bpt_io(s, cpu_tmp2_i32, ot);
+gen_bpt_io(s, s->tmp2_i32, ot);
 if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
 gen_io_end();
 }
@@ -1421,7 +1422,7 @@ static void gen_shift_flags(DisasContext *s, TCGMemOp ot, 
TCGv result,
 tcg_temp_free(z_tl);
 
 /* Get the two potential CC_OP values into temporaries.  */
-tcg_gen_movi_i32(cpu_tmp2_i32, (is_right ? CC_OP_SARB : CC_OP_SHLB) + ot);
+tcg_gen_movi_i32(s->tmp2_i32, (is_right ? CC_OP_SARB : CC_OP_SHLB) + ot);
 if (s->cc_op == CC_OP_DYNAMIC) {
 oldop = cpu_cc_op;
 } else {
@@ -1433,7 +1434,7 @@ static void gen_shift_flags(DisasContext *s, TCGMemOp ot, 
TCGv result,
 z32 = tcg_const_i32(0);
 s32 = tcg_temp_new_i32();
 tcg_gen_trunc_tl_i32(s32, count);
-tcg_gen_movcond_i32(TCG_COND_NE, cpu_cc_op, s32, z32, cpu_tmp2_i32, oldop);
+tcg_gen_movcond_i32(TCG_COND_NE, cpu_cc_op, s32, z32, s->tmp2_i32, oldop);
 tcg_temp_free_i32(z32);
 tcg_temp_free_i32(s32);
 
@@ -1544,14 +1545,14 @@ static void gen_rot_rm_T1(DisasContext *s, TCGMemOp ot, 
int op1, int is_right)
 do_long:
 #ifdef TARGET_X86_64
 case MO_32:
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
+tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
 tcg_gen_trunc_tl_i32(cpu_tmp3_i32, s->T1);
 if (is_right) {
-tcg_gen_rotr_i32(cpu_tmp2_i32, cpu_tmp2_i32, cpu_tmp3_i32);
+tcg_gen_rotr_i32(s->tmp2_i32, s->tmp2_i32, cpu_t

[Qemu-devel] [PATCH v3 11/13] target/i386: move cpu_tmp1_i64 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 160 
 1 file changed, 80 insertions(+), 80 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index cd880cc2a8..61a98ef872 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,8 +79,6 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv_i64 cpu_tmp1_i64;
-
 #include "exec/gen-icount.h"
 
 #ifdef TARGET_X86_64
@@ -143,6 +141,7 @@ typedef struct DisasContext {
 TCGv_ptr ptr1;
 TCGv_i32 tmp2_i32;
 TCGv_i32 tmp3_i32;
+TCGv_i64 tmp1_i64;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -2107,12 +2106,12 @@ static void gen_bndck(CPUX86State *env, DisasContext 
*s, int modrm,
 {
 TCGv ea = gen_lea_modrm_1(s, gen_lea_modrm_0(env, s, modrm));
 
-tcg_gen_extu_tl_i64(cpu_tmp1_i64, ea);
+tcg_gen_extu_tl_i64(s->tmp1_i64, ea);
 if (!CODE64(s)) {
-tcg_gen_ext32u_i64(cpu_tmp1_i64, cpu_tmp1_i64);
+tcg_gen_ext32u_i64(s->tmp1_i64, s->tmp1_i64);
 }
-tcg_gen_setcond_i64(cond, cpu_tmp1_i64, cpu_tmp1_i64, bndv);
-tcg_gen_extrl_i64_i32(s->tmp2_i32, cpu_tmp1_i64);
+tcg_gen_setcond_i64(cond, s->tmp1_i64, s->tmp1_i64, bndv);
+tcg_gen_extrl_i64_i32(s->tmp2_i32, s->tmp1_i64);
 gen_helper_bndck(cpu_env, s->tmp2_i32);
 }
 
@@ -2641,48 +2640,48 @@ static void gen_jmp(DisasContext *s, target_ulong eip)
 
 static inline void gen_ldq_env_A0(DisasContext *s, int offset)
 {
-tcg_gen_qemu_ld_i64(cpu_tmp1_i64, s->A0, s->mem_index, MO_LEQ);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, offset);
+tcg_gen_qemu_ld_i64(s->tmp1_i64, s->A0, s->mem_index, MO_LEQ);
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset);
 }
 
 static inline void gen_stq_env_A0(DisasContext *s, int offset)
 {
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, offset);
-tcg_gen_qemu_st_i64(cpu_tmp1_i64, s->A0, s->mem_index, MO_LEQ);
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, offset);
+tcg_gen_qemu_st_i64(s->tmp1_i64, s->A0, s->mem_index, MO_LEQ);
 }
 
 static inline void gen_ldo_env_A0(DisasContext *s, int offset)
 {
 int mem_index = s->mem_index;
-tcg_gen_qemu_ld_i64(cpu_tmp1_i64, s->A0, mem_index, MO_LEQ);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
+tcg_gen_qemu_ld_i64(s->tmp1_i64, s->A0, mem_index, MO_LEQ);
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
 tcg_gen_addi_tl(s->tmp0, s->A0, 8);
-tcg_gen_qemu_ld_i64(cpu_tmp1_i64, s->tmp0, mem_index, MO_LEQ);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(1)));
+tcg_gen_qemu_ld_i64(s->tmp1_i64, s->tmp0, mem_index, MO_LEQ);
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(1)));
 }
 
 static inline void gen_sto_env_A0(DisasContext *s, int offset)
 {
 int mem_index = s->mem_index;
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
-tcg_gen_qemu_st_i64(cpu_tmp1_i64, s->A0, mem_index, MO_LEQ);
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(0)));
+tcg_gen_qemu_st_i64(s->tmp1_i64, s->A0, mem_index, MO_LEQ);
 tcg_gen_addi_tl(s->tmp0, s->A0, 8);
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(1)));
-tcg_gen_qemu_st_i64(cpu_tmp1_i64, s->tmp0, mem_index, MO_LEQ);
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, offset + offsetof(ZMMReg, ZMM_Q(1)));
+tcg_gen_qemu_st_i64(s->tmp1_i64, s->tmp0, mem_index, MO_LEQ);
 }
 
-static inline void gen_op_movo(int d_offset, int s_offset)
+static inline void gen_op_movo(DisasContext *s, int d_offset, int s_offset)
 {
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset + offsetof(ZMMReg, 
ZMM_Q(0)));
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset + offsetof(ZMMReg, 
ZMM_Q(0)));
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset + offsetof(ZMMReg, 
ZMM_Q(1)));
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset + offsetof(ZMMReg, 
ZMM_Q(1)));
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, s_offset + offsetof(ZMMReg, 
ZMM_Q(0)));
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, d_offset + offsetof(ZMMReg, 
ZMM_Q(0)));
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, s_offset + offsetof(ZMMReg, 
ZMM_Q(1)));
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, d_offset + offsetof(ZMMReg, 
ZMM_Q(1)));
 }
 
-static inline void gen_op_movq(int d_offset, int s_offset)
+static inline void gen_op_movq(DisasContext *s, int d_offset, int s_offset)
 {
-tcg_gen_ld_i64(cpu_tmp1_i64, cpu_env, s_offset);
-tcg_gen_st_i64(cpu_tmp1_i64, cpu_env, d_offset);
+tcg_gen_ld_i64(s->tmp1_i64, cpu_env, s_offset);
+tcg_gen_st_i64(s->tmp1_i64, cpu_env, d_offset);
 }
 
 static inline void gen_op_movl(DisasContext *s, int d_offset, int s_offset)
@@ -2691,10 +2690,10 @@ static inline void gen_op_movl(DisasContext *s, int 
d_offset, int s_offset)
 tcg_gen_st_i32(s->tmp2_i32, cpu_env, d_offset);
 }
 
-static inline void gen_op_movq_

[Qemu-devel] [PATCH v3 03/13] target/i386: move cpu_T0 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 1174 ---
 1 file changed, 594 insertions(+), 580 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index c6b1baab9d..73fd7e5b9a 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,7 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 /* local temps */
-static TCGv cpu_T0, cpu_T1;
+static TCGv cpu_T1;
 /* local register indexes (only used inside old micro ops) */
 static TCGv cpu_tmp0, cpu_tmp4;
 static TCGv_ptr cpu_ptr0, cpu_ptr1;
@@ -138,6 +138,7 @@ typedef struct DisasContext {
 /* TCG local temps */
 TCGv cc_srcT;
 TCGv A0;
+TCGv T0;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -412,9 +413,9 @@ static inline void gen_op_add_reg_im(TCGMemOp size, int 
reg, int32_t val)
 gen_op_mov_reg_v(size, reg, cpu_tmp0);
 }
 
-static inline void gen_op_add_reg_T0(TCGMemOp size, int reg)
+static inline void gen_op_add_reg_T0(DisasContext *s, TCGMemOp size, int reg)
 {
-tcg_gen_add_tl(cpu_tmp0, cpu_regs[reg], cpu_T0);
+tcg_gen_add_tl(cpu_tmp0, cpu_regs[reg], s->T0);
 gen_op_mov_reg_v(size, reg, cpu_tmp0);
 }
 
@@ -431,9 +432,9 @@ static inline void gen_op_st_v(DisasContext *s, int idx, 
TCGv t0, TCGv a0)
 static inline void gen_op_st_rm_T0_A0(DisasContext *s, int idx, int d)
 {
 if (d == OR_TMP0) {
-gen_op_st_v(s, idx, cpu_T0, s->A0);
+gen_op_st_v(s, idx, s->T0, s->A0);
 } else {
-gen_op_mov_reg_v(idx, d, cpu_T0);
+gen_op_mov_reg_v(idx, d, s->T0);
 }
 }
 
@@ -509,10 +510,10 @@ static inline void gen_string_movl_A0_EDI(DisasContext *s)
 gen_lea_v_seg(s, s->aflag, cpu_regs[R_EDI], R_ES, -1);
 }
 
-static inline void gen_op_movl_T0_Dshift(TCGMemOp ot)
+static inline void gen_op_movl_T0_Dshift(DisasContext *s, TCGMemOp ot)
 {
-tcg_gen_ld32s_tl(cpu_T0, cpu_env, offsetof(CPUX86State, df));
-tcg_gen_shli_tl(cpu_T0, cpu_T0, ot);
+tcg_gen_ld32s_tl(s->T0, cpu_env, offsetof(CPUX86State, df));
+tcg_gen_shli_tl(s->T0, s->T0, ot);
 };
 
 static TCGv gen_ext_tl(TCGv dst, TCGv src, TCGMemOp size, bool sign)
@@ -610,7 +611,7 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 target_ulong next_eip;
 
 if (s->pe && (s->cpl > s->iopl || s->vm86)) {
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T0);
+tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
 switch (ot) {
 case MO_8:
 gen_helper_check_iob(cpu_env, cpu_tmp2_i32);
@@ -630,7 +631,7 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 gen_jmp_im(cur_eip);
 svm_flags |= (1 << (4 + ot));
 next_eip = s->pc - s->cs_base;
-tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_T0);
+tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
 gen_helper_svm_check_io(cpu_env, cpu_tmp2_i32,
 tcg_const_i32(svm_flags),
 tcg_const_i32(next_eip - cur_eip));
@@ -640,41 +641,41 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 static inline void gen_movs(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_ESI(s);
-gen_op_ld_v(s, ot, cpu_T0, s->A0);
+gen_op_ld_v(s, ot, s->T0, s->A0);
 gen_string_movl_A0_EDI(s);
-gen_op_st_v(s, ot, cpu_T0, s->A0);
-gen_op_movl_T0_Dshift(ot);
-gen_op_add_reg_T0(s->aflag, R_ESI);
-gen_op_add_reg_T0(s->aflag, R_EDI);
+gen_op_st_v(s, ot, s->T0, s->A0);
+gen_op_movl_T0_Dshift(s, ot);
+gen_op_add_reg_T0(s, s->aflag, R_ESI);
+gen_op_add_reg_T0(s, s->aflag, R_EDI);
 }
 
-static void gen_op_update1_cc(void)
+static void gen_op_update1_cc(DisasContext *s)
 {
-tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
+tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 }
 
-static void gen_op_update2_cc(void)
+static void gen_op_update2_cc(DisasContext *s)
 {
 tcg_gen_mov_tl(cpu_cc_src, cpu_T1);
-tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
+tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 }
 
-static void gen_op_update3_cc(TCGv reg)
+static void gen_op_update3_cc(DisasContext *s, TCGv reg)
 {
 tcg_gen_mov_tl(cpu_cc_src2, reg);
 tcg_gen_mov_tl(cpu_cc_src, cpu_T1);
-tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
+tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 }
 
-static inline void gen_op_testl_T0_T1_cc(void)
+static inline void gen_op_testl_T0_T1_cc(DisasContext *s)
 {
-tcg_gen_and_tl(cpu_cc_dst, cpu_T0, cpu_T1);
+tcg_gen_and_tl(cpu_cc_dst, s->T0, cpu_T1);
 }
 
 static void gen_op_update_neg_cc(DisasContext *s)
 {
-tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
-tcg_gen_neg_tl(cpu_cc_src, cpu_T0);
+tcg_gen_mov_tl(cpu_cc_dst, s->T0);
+tcg_gen_neg_tl(cpu_cc_src, s->T0);
 tcg_gen_movi_tl(s->cc_srcT, 0);
 }
 
@@ -1022,11 +1023,11 @@ static inline void gen_compute_eflags_c(DisasContext 
*s, TCGv reg)
value 'b'. In the fast case, T0 is guaranted not to be used. */
 s

[Qemu-devel] [PATCH v3 12/13] target/i386: move x86_64_hregs to DisasContext

And convert it to a bool to use an existing hole
in the struct.

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 307 
 1 file changed, 154 insertions(+), 153 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 61a98ef872..b8222dc4ba 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -81,10 +81,6 @@ static TCGv_i64 cpu_bndu[4];
 
 #include "exec/gen-icount.h"
 
-#ifdef TARGET_X86_64
-static int x86_64_hregs;
-#endif
-
 typedef struct DisasContext {
 DisasContextBase base;
 
@@ -109,6 +105,9 @@ typedef struct DisasContext {
 int ss32;   /* 32 bit stack segment */
 CCOp cc_op;  /* current CC operation */
 bool cc_op_dirty;
+#ifdef TARGET_X86_64
+bool x86_64_hregs;
+#endif
 int addseg; /* non zero if either DS/ES/SS have a non zero base */
 int f_st;   /* currently unused */
 int vm86;   /* vm86 mode */
@@ -307,13 +306,13 @@ static void gen_update_cc_op(DisasContext *s)
  * [AH, CH, DH, BH], ie "bits 15..8 of register N-4". Return
  * true for this special case, false otherwise.
  */
-static inline bool byte_reg_is_xH(int reg)
+static inline bool byte_reg_is_xH(DisasContext *s, int reg)
 {
 if (reg < 4) {
 return false;
 }
 #ifdef TARGET_X86_64
-if (reg >= 8 || x86_64_hregs) {
+if (reg >= 8 || s->x86_64_hregs) {
 return false;
 }
 #endif
@@ -360,11 +359,11 @@ static inline TCGMemOp mo_b_d32(int b, TCGMemOp ot)
 return b & 1 ? (ot == MO_16 ? MO_16 : MO_32) : MO_8;
 }
 
-static void gen_op_mov_reg_v(TCGMemOp ot, int reg, TCGv t0)
+static void gen_op_mov_reg_v(DisasContext *s, TCGMemOp ot, int reg, TCGv t0)
 {
 switch(ot) {
 case MO_8:
-if (!byte_reg_is_xH(reg)) {
+if (!byte_reg_is_xH(s, reg)) {
 tcg_gen_deposit_tl(cpu_regs[reg], cpu_regs[reg], t0, 0, 8);
 } else {
 tcg_gen_deposit_tl(cpu_regs[reg - 4], cpu_regs[reg - 4], t0, 8, 8);
@@ -388,9 +387,10 @@ static void gen_op_mov_reg_v(TCGMemOp ot, int reg, TCGv t0)
 }
 }
 
-static inline void gen_op_mov_v_reg(TCGMemOp ot, TCGv t0, int reg)
+static inline
+void gen_op_mov_v_reg(DisasContext *s, TCGMemOp ot, TCGv t0, int reg)
 {
-if (ot == MO_8 && byte_reg_is_xH(reg)) {
+if (ot == MO_8 && byte_reg_is_xH(s, reg)) {
 tcg_gen_extract_tl(t0, cpu_regs[reg - 4], 8, 8);
 } else {
 tcg_gen_mov_tl(t0, cpu_regs[reg]);
@@ -414,13 +414,13 @@ static inline
 void gen_op_add_reg_im(DisasContext *s, TCGMemOp size, int reg, int32_t val)
 {
 tcg_gen_addi_tl(s->tmp0, cpu_regs[reg], val);
-gen_op_mov_reg_v(size, reg, s->tmp0);
+gen_op_mov_reg_v(s, size, reg, s->tmp0);
 }
 
 static inline void gen_op_add_reg_T0(DisasContext *s, TCGMemOp size, int reg)
 {
 tcg_gen_add_tl(s->tmp0, cpu_regs[reg], s->T0);
-gen_op_mov_reg_v(size, reg, s->tmp0);
+gen_op_mov_reg_v(s, size, reg, s->tmp0);
 }
 
 static inline void gen_op_ld_v(DisasContext *s, int idx, TCGv t0, TCGv a0)
@@ -438,7 +438,7 @@ static inline void gen_op_st_rm_T0_A0(DisasContext *s, int 
idx, int d)
 if (d == OR_TMP0) {
 gen_op_st_v(s, idx, s->T0, s->A0);
 } else {
-gen_op_mov_reg_v(idx, d, s->T0);
+gen_op_mov_reg_v(s, idx, d, s->T0);
 }
 }
 
@@ -1077,7 +1077,7 @@ static TCGLabel *gen_jz_ecx_string(DisasContext *s, 
target_ulong next_eip)
 
 static inline void gen_stos(DisasContext *s, TCGMemOp ot)
 {
-gen_op_mov_v_reg(MO_32, s->T0, R_EAX);
+gen_op_mov_v_reg(s, MO_32, s->T0, R_EAX);
 gen_string_movl_A0_EDI(s);
 gen_op_st_v(s, ot, s->T0, s->A0);
 gen_op_movl_T0_Dshift(s, ot);
@@ -1088,7 +1088,7 @@ static inline void gen_lods(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, s->T0, s->A0);
-gen_op_mov_reg_v(ot, R_EAX, s->T0);
+gen_op_mov_reg_v(s, ot, R_EAX, s->T0);
 gen_op_movl_T0_Dshift(s, ot);
 gen_op_add_reg_T0(s, s->aflag, R_ESI);
 }
@@ -1272,7 +1272,7 @@ static void gen_helper_fp_arith_STN_ST0(int op, int opreg)
 static void gen_op(DisasContext *s1, int op, TCGMemOp ot, int d)
 {
 if (d != OR_TMP0) {
-gen_op_mov_v_reg(ot, s1->T0, d);
+gen_op_mov_v_reg(s1, ot, s1->T0, d);
 } else if (!(s1->prefix & PREFIX_LOCK)) {
 gen_op_ld_v(s1, ot, s1->T0, s1->A0);
 }
@@ -1383,7 +1383,7 @@ static void gen_inc(DisasContext *s1, TCGMemOp ot, int d, 
int c)
 s1->mem_index, ot | MO_LE);
 } else {
 if (d != OR_TMP0) {
-gen_op_mov_v_reg(ot, s1->T0, d);
+gen_op_mov_v_reg(s1, ot, s1->T0, d);
 } else {
 gen_op_ld_v(s1, ot, s1->T0, s1->A0);
 }
@@ -1450,7 +1450,7 @@ static void gen_shift_rm_T1(DisasContext *s, TCGMemOp ot, 
int op1,
 if (op1 == OR_TMP0) {
 gen_op_ld_v(s, ot, s->T0, s->A0);
 } else {
-gen_op_mov_v_reg(ot, s->T0, op1);
+gen_op_mov_v_reg(s, ot, s->T0, op1);
 }
 
 tcg_gen_andi_tl(s->

[Qemu-devel] [PATCH v3 05/13] target/i386: move cpu_tmp0 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 282 
 1 file changed, 144 insertions(+), 138 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index bd27e65344..873231fb44 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -78,8 +78,8 @@ static TCGv cpu_regs[CPU_NB_REGS];
 static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
-/* local register indexes (only used inside old micro ops) */
-static TCGv cpu_tmp0, cpu_tmp4;
+
+static TCGv cpu_tmp4;
 static TCGv_ptr cpu_ptr0, cpu_ptr1;
 static TCGv_i32 cpu_tmp2_i32, cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
@@ -139,6 +139,9 @@ typedef struct DisasContext {
 TCGv T0;
 TCGv T1;
 
+/* TCG local register indexes (only used inside old micro ops) */
+TCGv tmp0;
+
 sigjmp_buf jmpbuf;
 } DisasContext;
 
@@ -406,16 +409,17 @@ static inline void gen_op_jmp_v(TCGv dest)
 tcg_gen_st_tl(dest, cpu_env, offsetof(CPUX86State, eip));
 }
 
-static inline void gen_op_add_reg_im(TCGMemOp size, int reg, int32_t val)
+static inline
+void gen_op_add_reg_im(DisasContext *s, TCGMemOp size, int reg, int32_t val)
 {
-tcg_gen_addi_tl(cpu_tmp0, cpu_regs[reg], val);
-gen_op_mov_reg_v(size, reg, cpu_tmp0);
+tcg_gen_addi_tl(s->tmp0, cpu_regs[reg], val);
+gen_op_mov_reg_v(size, reg, s->tmp0);
 }
 
 static inline void gen_op_add_reg_T0(DisasContext *s, TCGMemOp size, int reg)
 {
-tcg_gen_add_tl(cpu_tmp0, cpu_regs[reg], s->T0);
-gen_op_mov_reg_v(size, reg, cpu_tmp0);
+tcg_gen_add_tl(s->tmp0, cpu_regs[reg], s->T0);
+gen_op_mov_reg_v(size, reg, s->tmp0);
 }
 
 static inline void gen_op_ld_v(DisasContext *s, int idx, TCGv t0, TCGv a0)
@@ -437,10 +441,10 @@ static inline void gen_op_st_rm_T0_A0(DisasContext *s, 
int idx, int d)
 }
 }
 
-static inline void gen_jmp_im(target_ulong pc)
+static inline void gen_jmp_im(DisasContext *s, target_ulong pc)
 {
-tcg_gen_movi_tl(cpu_tmp0, pc);
-gen_op_jmp_v(cpu_tmp0);
+tcg_gen_movi_tl(s->tmp0, pc);
+gen_op_jmp_v(s->tmp0);
 }
 
 /* Compute SEG:REG into A0.  SEG is selected from the override segment
@@ -556,18 +560,20 @@ static void gen_exts(TCGMemOp ot, TCGv reg)
 gen_ext_tl(reg, reg, ot, true);
 }
 
-static inline void gen_op_jnz_ecx(TCGMemOp size, TCGLabel *label1)
+static inline
+void gen_op_jnz_ecx(DisasContext *s, TCGMemOp size, TCGLabel *label1)
 {
-tcg_gen_mov_tl(cpu_tmp0, cpu_regs[R_ECX]);
-gen_extu(size, cpu_tmp0);
-tcg_gen_brcondi_tl(TCG_COND_NE, cpu_tmp0, 0, label1);
+tcg_gen_mov_tl(s->tmp0, cpu_regs[R_ECX]);
+gen_extu(size, s->tmp0);
+tcg_gen_brcondi_tl(TCG_COND_NE, s->tmp0, 0, label1);
 }
 
-static inline void gen_op_jz_ecx(TCGMemOp size, TCGLabel *label1)
+static inline
+void gen_op_jz_ecx(DisasContext *s, TCGMemOp size, TCGLabel *label1)
 {
-tcg_gen_mov_tl(cpu_tmp0, cpu_regs[R_ECX]);
-gen_extu(size, cpu_tmp0);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_tmp0, 0, label1);
+tcg_gen_mov_tl(s->tmp0, cpu_regs[R_ECX]);
+gen_extu(size, s->tmp0);
+tcg_gen_brcondi_tl(TCG_COND_EQ, s->tmp0, 0, label1);
 }
 
 static void gen_helper_in_func(TCGMemOp ot, TCGv v, TCGv_i32 n)
@@ -627,7 +633,7 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 }
 if(s->flags & HF_SVMI_MASK) {
 gen_update_cc_op(s);
-gen_jmp_im(cur_eip);
+gen_jmp_im(s, cur_eip);
 svm_flags |= (1 << (4 + ot));
 next_eip = s->pc - s->cs_base;
 tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
@@ -743,9 +749,9 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv 
reg)
 case CC_OP_SUBB ... CC_OP_SUBQ:
 /* (DATA_TYPE)CC_SRCT < (DATA_TYPE)CC_SRC */
 size = s->cc_op - CC_OP_SUBB;
-t1 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, false);
+t1 = gen_ext_tl(s->tmp0, cpu_cc_src, size, false);
 /* If no temporary was used, be careful not to alias t1 and t0.  */
-t0 = t1 == cpu_cc_src ? cpu_tmp0 : reg;
+t0 = t1 == cpu_cc_src ? s->tmp0 : reg;
 tcg_gen_mov_tl(t0, s->cc_srcT);
 gen_extu(size, t0);
 goto add_sub;
@@ -753,7 +759,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv 
reg)
 case CC_OP_ADDB ... CC_OP_ADDQ:
 /* (DATA_TYPE)CC_DST < (DATA_TYPE)CC_SRC */
 size = s->cc_op - CC_OP_ADDB;
-t1 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, false);
+t1 = gen_ext_tl(s->tmp0, cpu_cc_src, size, false);
 t0 = gen_ext_tl(reg, cpu_cc_dst, size, false);
 add_sub:
 return (CCPrepare) { .cond = TCG_COND_LTU, .reg = t0,
@@ -905,7 +911,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, 
TCGv reg)
 case JCC_BE:
 tcg_gen_mov_tl(cpu_tmp4, s->cc_srcT);
 gen_extu(size, cpu_tmp4);
-t0 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, false);
+t0 = gen_ext_tl(s->tmp0, cpu_cc_src, size, false);

[Qemu-devel] [PATCH v3 02/13] target/i386: move cpu_A0 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 472 
 1 file changed, 236 insertions(+), 236 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index e9f512472e..c6b1baab9d 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -72,7 +72,6 @@
 //#define MACRO_TEST   1
 
 /* global register indexes */
-static TCGv cpu_A0;
 static TCGv cpu_cc_dst, cpu_cc_src, cpu_cc_src2;
 static TCGv_i32 cpu_cc_op;
 static TCGv cpu_regs[CPU_NB_REGS];
@@ -138,6 +137,7 @@ typedef struct DisasContext {
 
 /* TCG local temps */
 TCGv cc_srcT;
+TCGv A0;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -395,9 +395,9 @@ static inline void gen_op_mov_v_reg(TCGMemOp ot, TCGv t0, 
int reg)
 
 static void gen_add_A0_im(DisasContext *s, int val)
 {
-tcg_gen_addi_tl(cpu_A0, cpu_A0, val);
+tcg_gen_addi_tl(s->A0, s->A0, val);
 if (!CODE64(s)) {
-tcg_gen_ext32u_tl(cpu_A0, cpu_A0);
+tcg_gen_ext32u_tl(s->A0, s->A0);
 }
 }
 
@@ -431,7 +431,7 @@ static inline void gen_op_st_v(DisasContext *s, int idx, 
TCGv t0, TCGv a0)
 static inline void gen_op_st_rm_T0_A0(DisasContext *s, int idx, int d)
 {
 if (d == OR_TMP0) {
-gen_op_st_v(s, idx, cpu_T0, cpu_A0);
+gen_op_st_v(s, idx, cpu_T0, s->A0);
 } else {
 gen_op_mov_reg_v(idx, d, cpu_T0);
 }
@@ -453,7 +453,7 @@ static void gen_lea_v_seg(DisasContext *s, TCGMemOp aflag, 
TCGv a0,
 #ifdef TARGET_X86_64
 case MO_64:
 if (ovr_seg < 0) {
-tcg_gen_mov_tl(cpu_A0, a0);
+tcg_gen_mov_tl(s->A0, a0);
 return;
 }
 break;
@@ -464,14 +464,14 @@ static void gen_lea_v_seg(DisasContext *s, TCGMemOp 
aflag, TCGv a0,
 ovr_seg = def_seg;
 }
 if (ovr_seg < 0) {
-tcg_gen_ext32u_tl(cpu_A0, a0);
+tcg_gen_ext32u_tl(s->A0, a0);
 return;
 }
 break;
 case MO_16:
 /* 16 bit address */
-tcg_gen_ext16u_tl(cpu_A0, a0);
-a0 = cpu_A0;
+tcg_gen_ext16u_tl(s->A0, a0);
+a0 = s->A0;
 if (ovr_seg < 0) {
 if (s->addseg) {
 ovr_seg = def_seg;
@@ -488,13 +488,13 @@ static void gen_lea_v_seg(DisasContext *s, TCGMemOp 
aflag, TCGv a0,
 TCGv seg = cpu_seg_base[ovr_seg];
 
 if (aflag == MO_64) {
-tcg_gen_add_tl(cpu_A0, a0, seg);
+tcg_gen_add_tl(s->A0, a0, seg);
 } else if (CODE64(s)) {
-tcg_gen_ext32u_tl(cpu_A0, a0);
-tcg_gen_add_tl(cpu_A0, cpu_A0, seg);
+tcg_gen_ext32u_tl(s->A0, a0);
+tcg_gen_add_tl(s->A0, s->A0, seg);
 } else {
-tcg_gen_add_tl(cpu_A0, a0, seg);
-tcg_gen_ext32u_tl(cpu_A0, cpu_A0);
+tcg_gen_add_tl(s->A0, a0, seg);
+tcg_gen_ext32u_tl(s->A0, s->A0);
 }
 }
 }
@@ -640,9 +640,9 @@ static void gen_check_io(DisasContext *s, TCGMemOp ot, 
target_ulong cur_eip,
 static inline void gen_movs(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_ESI(s);
-gen_op_ld_v(s, ot, cpu_T0, cpu_A0);
+gen_op_ld_v(s, ot, cpu_T0, s->A0);
 gen_string_movl_A0_EDI(s);
-gen_op_st_v(s, ot, cpu_T0, cpu_A0);
+gen_op_st_v(s, ot, cpu_T0, s->A0);
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_ESI);
 gen_op_add_reg_T0(s->aflag, R_EDI);
@@ -1072,7 +1072,7 @@ static inline void gen_stos(DisasContext *s, TCGMemOp ot)
 {
 gen_op_mov_v_reg(MO_32, cpu_T0, R_EAX);
 gen_string_movl_A0_EDI(s);
-gen_op_st_v(s, ot, cpu_T0, cpu_A0);
+gen_op_st_v(s, ot, cpu_T0, s->A0);
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_EDI);
 }
@@ -1080,7 +1080,7 @@ static inline void gen_stos(DisasContext *s, TCGMemOp ot)
 static inline void gen_lods(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_ESI(s);
-gen_op_ld_v(s, ot, cpu_T0, cpu_A0);
+gen_op_ld_v(s, ot, cpu_T0, s->A0);
 gen_op_mov_reg_v(ot, R_EAX, cpu_T0);
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_ESI);
@@ -1089,7 +1089,7 @@ static inline void gen_lods(DisasContext *s, TCGMemOp ot)
 static inline void gen_scas(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_EDI(s);
-gen_op_ld_v(s, ot, cpu_T1, cpu_A0);
+gen_op_ld_v(s, ot, cpu_T1, s->A0);
 gen_op(s, OP_CMPL, ot, R_EAX);
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_EDI);
@@ -1098,7 +1098,7 @@ static inline void gen_scas(DisasContext *s, TCGMemOp ot)
 static inline void gen_cmps(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_EDI(s);
-gen_op_ld_v(s, ot, cpu_T1, cpu_A0);
+gen_op_ld_v(s, ot, cpu_T1, s->A0);
 gen_string_movl_A0_ESI(s);
 gen_op(s, OP_CMPL, ot, OR_TMP0);
 gen_op_movl_T0_Dshift(ot);
@@ -1128,11 +1128,11 @@ static inline void gen_ins(DisasContext *s, TCGMemOp ot)
 /* Note: we must do this dummy write first to be restartable in

[Qemu-devel] [PATCH v3 13/13] configure: enable mttcg for i386 and x86_64

Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index 58862d2ae8..f715252c9f 100755
--- a/configure
+++ b/configure
@@ -7025,12 +7025,14 @@ TARGET_ABI_DIR=""
 
 case "$target_name" in
   i386)
+mttcg="yes"
 gdb_xml_files="i386-32bit.xml i386-32bit-core.xml i386-32bit-sse.xml"
 target_compiler=$cross_cc_i386
 target_compiler_cflags=$cross_cc_ccflags_i386
   ;;
   x86_64)
 TARGET_BASE_ARCH=i386
+mttcg="yes"
 gdb_xml_files="i386-64bit.xml i386-64bit-core.xml i386-64bit-sse.xml"
 target_compiler=$cross_cc_x86_64
   ;;
-- 
2.17.1

[Qemu-devel] [PATCH v3 06/13] target/i386: move cpu_tmp4 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 78 -
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 873231fb44..0ad6ffc4af 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,6 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv cpu_tmp4;
 static TCGv_ptr cpu_ptr0, cpu_ptr1;
 static TCGv_i32 cpu_tmp2_i32, cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
@@ -141,6 +140,7 @@ typedef struct DisasContext {
 
 /* TCG local register indexes (only used inside old micro ops) */
 TCGv tmp0;
+TCGv tmp4;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -909,10 +909,10 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, 
TCGv reg)
 size = s->cc_op - CC_OP_SUBB;
 switch (jcc_op) {
 case JCC_BE:
-tcg_gen_mov_tl(cpu_tmp4, s->cc_srcT);
-gen_extu(size, cpu_tmp4);
+tcg_gen_mov_tl(s->tmp4, s->cc_srcT);
+gen_extu(size, s->tmp4);
 t0 = gen_ext_tl(s->tmp0, cpu_cc_src, size, false);
-cc = (CCPrepare) { .cond = TCG_COND_LEU, .reg = cpu_tmp4,
+cc = (CCPrepare) { .cond = TCG_COND_LEU, .reg = s->tmp4,
.reg2 = t0, .mask = -1, .use_reg2 = true };
 break;
 
@@ -922,10 +922,10 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, 
TCGv reg)
 case JCC_LE:
 cond = TCG_COND_LE;
 fast_jcc_l:
-tcg_gen_mov_tl(cpu_tmp4, s->cc_srcT);
-gen_exts(size, cpu_tmp4);
+tcg_gen_mov_tl(s->tmp4, s->cc_srcT);
+gen_exts(size, s->tmp4);
 t0 = gen_ext_tl(s->tmp0, cpu_cc_src, size, true);
-cc = (CCPrepare) { .cond = cond, .reg = cpu_tmp4,
+cc = (CCPrepare) { .cond = cond, .reg = s->tmp4,
.reg2 = t0, .mask = -1, .use_reg2 = true };
 break;
 
@@ -1277,32 +1277,32 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 }
 switch(op) {
 case OP_ADCL:
-gen_compute_eflags_c(s1, cpu_tmp4);
+gen_compute_eflags_c(s1, s1->tmp4);
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_add_tl(s1->T0, cpu_tmp4, s1->T1);
+tcg_gen_add_tl(s1->T0, s1->tmp4, s1->T1);
 tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
 s1->mem_index, ot | MO_LE);
 } else {
 tcg_gen_add_tl(s1->T0, s1->T0, s1->T1);
-tcg_gen_add_tl(s1->T0, s1->T0, cpu_tmp4);
+tcg_gen_add_tl(s1->T0, s1->T0, s1->tmp4);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
-gen_op_update3_cc(s1, cpu_tmp4);
+gen_op_update3_cc(s1, s1->tmp4);
 set_cc_op(s1, CC_OP_ADCB + ot);
 break;
 case OP_SBBL:
-gen_compute_eflags_c(s1, cpu_tmp4);
+gen_compute_eflags_c(s1, s1->tmp4);
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_add_tl(s1->T0, s1->T1, cpu_tmp4);
+tcg_gen_add_tl(s1->T0, s1->T1, s1->tmp4);
 tcg_gen_neg_tl(s1->T0, s1->T0);
 tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
 s1->mem_index, ot | MO_LE);
 } else {
 tcg_gen_sub_tl(s1->T0, s1->T0, s1->T1);
-tcg_gen_sub_tl(s1->T0, s1->T0, cpu_tmp4);
+tcg_gen_sub_tl(s1->T0, s1->T0, s1->tmp4);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
-gen_op_update3_cc(s1, cpu_tmp4);
+gen_op_update3_cc(s1, s1->tmp4);
 set_cc_op(s1, CC_OP_SBBB + ot);
 break;
 case OP_ADDL:
@@ -1492,15 +1492,15 @@ static void gen_shift_rm_im(DisasContext *s, TCGMemOp 
ot, int op1, int op2,
 if (is_right) {
 if (is_arith) {
 gen_exts(ot, s->T0);
-tcg_gen_sari_tl(cpu_tmp4, s->T0, op2 - 1);
+tcg_gen_sari_tl(s->tmp4, s->T0, op2 - 1);
 tcg_gen_sari_tl(s->T0, s->T0, op2);
 } else {
 gen_extu(ot, s->T0);
-tcg_gen_shri_tl(cpu_tmp4, s->T0, op2 - 1);
+tcg_gen_shri_tl(s->tmp4, s->T0, op2 - 1);
 tcg_gen_shri_tl(s->T0, s->T0, op2);
 }
 } else {
-tcg_gen_shli_tl(cpu_tmp4, s->T0, op2 - 1);
+tcg_gen_shli_tl(s->tmp4, s->T0, op2 - 1);
 tcg_gen_shli_tl(s->T0, s->T0, op2);
 }
 }
@@ -1510,7 +1510,7 @@ static void gen_shift_rm_im(DisasContext *s, TCGMemOp ot, 
int op1, int op2,
 
 /* update eflags if non zero shift */
 if (op2 != 0) {
-tcg_gen_mov_tl(cpu_cc_src, cpu_tmp4);
+tcg_gen_mov_tl(cpu_cc_src, s->tmp4);
 tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 set_cc_op(s, (is_right ? CC_OP_SARB : CC_OP_SHLB) + ot);
 }
@@ -1786,25 +1786,25 @@ static void gen_shiftd_

[Qemu-devel] [PATCH v3 08/13] target/i386: move cpu_ptr1 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 52 -
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 9531dafebe..c51f61ca2c 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,6 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv_ptr cpu_ptr1;
 static TCGv_i32 cpu_tmp2_i32, cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
 
@@ -142,6 +141,7 @@ typedef struct DisasContext {
 TCGv tmp0;
 TCGv tmp4;
 TCGv_ptr ptr0;
+TCGv_ptr ptr1;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -3473,8 +3473,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 op2_offset = offsetof(CPUX86State,fpregs[rm].mmx);
 }
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op2_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op1_offset);
-sse_fn_epp(cpu_env, s->ptr0, cpu_ptr1);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op1_offset);
+sse_fn_epp(cpu_env, s->ptr0, s->ptr1);
 break;
 case 0x050: /* movmskps */
 rm = (modrm & 7) | REX_B(s);
@@ -3503,14 +3503,14 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 }
 op1_offset = offsetof(CPUX86State,xmm_regs[reg]);
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
 switch(b >> 8) {
 case 0x0:
-gen_helper_cvtpi2ps(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvtpi2ps(cpu_env, s->ptr0, s->ptr1);
 break;
 default:
 case 0x1:
-gen_helper_cvtpi2pd(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvtpi2pd(cpu_env, s->ptr0, s->ptr1);
 break;
 }
 break;
@@ -3548,19 +3548,19 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 }
 op1_offset = offsetof(CPUX86State,fpregs[reg & 7].mmx);
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
 switch(b) {
 case 0x02c:
-gen_helper_cvttps2pi(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvttps2pi(cpu_env, s->ptr0, s->ptr1);
 break;
 case 0x12c:
-gen_helper_cvttpd2pi(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvttpd2pi(cpu_env, s->ptr0, s->ptr1);
 break;
 case 0x02d:
-gen_helper_cvtps2pi(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvtps2pi(cpu_env, s->ptr0, s->ptr1);
 break;
 case 0x12d:
-gen_helper_cvtpd2pi(cpu_env, s->ptr0, cpu_ptr1);
+gen_helper_cvtpd2pi(cpu_env, s->ptr0, s->ptr1);
 break;
 }
 break;
@@ -3749,8 +3749,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 }
 
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
-sse_fn_epp(cpu_env, s->ptr0, cpu_ptr1);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
+sse_fn_epp(cpu_env, s->ptr0, s->ptr1);
 
 if (b == 0x17) {
 set_cc_op(s, CC_OP_EFLAGS);
@@ -4298,8 +4298,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 }
 
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
-sse_fn_eppi(cpu_env, s->ptr0, cpu_ptr1, tcg_const_i32(val));
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
+sse_fn_eppi(cpu_env, s->ptr0, s->ptr1, tcg_const_i32(val));
 break;
 
 case 0x33a:
@@ -4421,17 +4421,17 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 goto illegal_op;
 }
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
-sse_fn_epp(cpu_env, s->ptr0, cpu_ptr1);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
+sse_fn_epp(cpu_env, s->ptr0, s->ptr1);
 break;
 case 0x70: /* pshufx insn */
 case 0xc6: /* pshufx insn */
 val = x86_ldub_code(env, s);
 tcg_gen_addi_ptr(s->ptr0, cpu_env, op1_offset);
-tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
+tcg_gen_addi_ptr(s->ptr1, cpu_env, op2_offset);
 /* XXX: introduce a new table? */
 sse_fn_ppi = (SSEFunc_0_ppi)sse_fn_epp;
-sse_fn_ppi(s

[Qemu-devel] [PATCH v3 07/13] target/i386: move cpu_ptr0 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 101 +---
 1 file changed, 52 insertions(+), 49 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 0ad6ffc4af..9531dafebe 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,7 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv_ptr cpu_ptr0, cpu_ptr1;
+static TCGv_ptr cpu_ptr1;
 static TCGv_i32 cpu_tmp2_i32, cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
 
@@ -141,6 +141,7 @@ typedef struct DisasContext {
 /* TCG local register indexes (only used inside old micro ops) */
 TCGv tmp0;
 TCGv tmp4;
+TCGv_ptr ptr0;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -3147,27 +3148,27 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 #endif
 {
 gen_ldst_modrm(env, s, modrm, MO_32, OR_TMP0, 0);
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
  offsetof(CPUX86State,fpregs[reg].mmx));
 tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
-gen_helper_movl_mm_T0_mmx(cpu_ptr0, cpu_tmp2_i32);
+gen_helper_movl_mm_T0_mmx(s->ptr0, cpu_tmp2_i32);
 }
 break;
 case 0x16e: /* movd xmm, ea */
 #ifdef TARGET_X86_64
 if (s->dflag == MO_64) {
 gen_ldst_modrm(env, s, modrm, MO_64, OR_TMP0, 0);
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
  offsetof(CPUX86State,xmm_regs[reg]));
-gen_helper_movq_mm_T0_xmm(cpu_ptr0, s->T0);
+gen_helper_movq_mm_T0_xmm(s->ptr0, s->T0);
 } else
 #endif
 {
 gen_ldst_modrm(env, s, modrm, MO_32, OR_TMP0, 0);
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
  offsetof(CPUX86State,xmm_regs[reg]));
 tcg_gen_trunc_tl_i32(cpu_tmp2_i32, s->T0);
-gen_helper_movl_mm_T0_xmm(cpu_ptr0, cpu_tmp2_i32);
+gen_helper_movl_mm_T0_xmm(s->ptr0, cpu_tmp2_i32);
 }
 break;
 case 0x6f: /* movq mm, ea */
@@ -3312,14 +3313,14 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 goto illegal_op;
 field_length = x86_ldub_code(env, s) & 0x3F;
 bit_index = x86_ldub_code(env, s) & 0x3F;
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env,
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
 offsetof(CPUX86State,xmm_regs[reg]));
 if (b1 == 1)
-gen_helper_extrq_i(cpu_env, cpu_ptr0,
+gen_helper_extrq_i(cpu_env, s->ptr0,
tcg_const_i32(bit_index),
tcg_const_i32(field_length));
 else
-gen_helper_insertq_i(cpu_env, cpu_ptr0,
+gen_helper_insertq_i(cpu_env, s->ptr0,
  tcg_const_i32(bit_index),
  tcg_const_i32(field_length));
 }
@@ -3471,22 +3472,22 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 rm = (modrm & 7);
 op2_offset = offsetof(CPUX86State,fpregs[rm].mmx);
 }
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op2_offset);
+tcg_gen_addi_ptr(s->ptr0, cpu_env, op2_offset);
 tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op1_offset);
-sse_fn_epp(cpu_env, cpu_ptr0, cpu_ptr1);
+sse_fn_epp(cpu_env, s->ptr0, cpu_ptr1);
 break;
 case 0x050: /* movmskps */
 rm = (modrm & 7) | REX_B(s);
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
  offsetof(CPUX86State,xmm_regs[rm]));
-gen_helper_movmskps(cpu_tmp2_i32, cpu_env, cpu_ptr0);
+gen_helper_movmskps(cpu_tmp2_i32, cpu_env, s->ptr0);
 tcg_gen_extu_i32_tl(cpu_regs[reg], cpu_tmp2_i32);
 break;
 case 0x150: /* movmskpd */
 rm = (modrm & 7) | REX_B(s);
-tcg_gen_addi_ptr(cpu_ptr0, cpu_env, 
+tcg_gen_addi_ptr(s->ptr0, cpu_env,
  offsetof(CPUX86State,xmm_regs[rm]));
-gen_helper_movmskpd(cpu_tmp2_i32, cpu_env, cpu_ptr0);
+gen_helper_movmskpd(cpu_tmp2_i32, cpu_env, s->ptr0);
 tcg_gen_extu_i32_tl(cpu_regs[reg], cpu_tmp2_i32);
 break;
 case 0x02a: /* cvtpi2ps */
@@ -3501,15 +3502,15 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 op2_offset = offsetof(CPUX86State,fpreg

[Qemu-devel] [PATCH v3 01/13] target/i386: move cpu_cc_srcT to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 1f9d1d9b24..e9f512472e 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -73,7 +73,7 @@
 
 /* global register indexes */
 static TCGv cpu_A0;
-static TCGv cpu_cc_dst, cpu_cc_src, cpu_cc_src2, cpu_cc_srcT;
+static TCGv cpu_cc_dst, cpu_cc_src, cpu_cc_src2;
 static TCGv_i32 cpu_cc_op;
 static TCGv cpu_regs[CPU_NB_REGS];
 static TCGv cpu_seg_base[6];
@@ -135,6 +135,10 @@ typedef struct DisasContext {
 int cpuid_ext3_features;
 int cpuid_7_0_ebx_features;
 int cpuid_xsave_features;
+
+/* TCG local temps */
+TCGv cc_srcT;
+
 sigjmp_buf jmpbuf;
 } DisasContext;
 
@@ -244,7 +248,7 @@ static void set_cc_op(DisasContext *s, CCOp op)
 tcg_gen_discard_tl(cpu_cc_src2);
 }
 if (dead & USES_CC_SRCT) {
-tcg_gen_discard_tl(cpu_cc_srcT);
+tcg_gen_discard_tl(s->cc_srcT);
 }
 
 if (op == CC_OP_DYNAMIC) {
@@ -667,11 +671,11 @@ static inline void gen_op_testl_T0_T1_cc(void)
 tcg_gen_and_tl(cpu_cc_dst, cpu_T0, cpu_T1);
 }
 
-static void gen_op_update_neg_cc(void)
+static void gen_op_update_neg_cc(DisasContext *s)
 {
 tcg_gen_mov_tl(cpu_cc_dst, cpu_T0);
 tcg_gen_neg_tl(cpu_cc_src, cpu_T0);
-tcg_gen_movi_tl(cpu_cc_srcT, 0);
+tcg_gen_movi_tl(s->cc_srcT, 0);
 }
 
 /* compute all eflags to cc_src */
@@ -742,7 +746,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv 
reg)
 t1 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, false);
 /* If no temporary was used, be careful not to alias t1 and t0.  */
 t0 = t1 == cpu_cc_src ? cpu_tmp0 : reg;
-tcg_gen_mov_tl(t0, cpu_cc_srcT);
+tcg_gen_mov_tl(t0, s->cc_srcT);
 gen_extu(size, t0);
 goto add_sub;
 
@@ -899,7 +903,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, 
TCGv reg)
 size = s->cc_op - CC_OP_SUBB;
 switch (jcc_op) {
 case JCC_BE:
-tcg_gen_mov_tl(cpu_tmp4, cpu_cc_srcT);
+tcg_gen_mov_tl(cpu_tmp4, s->cc_srcT);
 gen_extu(size, cpu_tmp4);
 t0 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, false);
 cc = (CCPrepare) { .cond = TCG_COND_LEU, .reg = cpu_tmp4,
@@ -912,7 +916,7 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b, 
TCGv reg)
 case JCC_LE:
 cond = TCG_COND_LE;
 fast_jcc_l:
-tcg_gen_mov_tl(cpu_tmp4, cpu_cc_srcT);
+tcg_gen_mov_tl(cpu_tmp4, s->cc_srcT);
 gen_exts(size, cpu_tmp4);
 t0 = gen_ext_tl(cpu_tmp0, cpu_cc_src, size, true);
 cc = (CCPrepare) { .cond = cond, .reg = cpu_tmp4,
@@ -1309,11 +1313,11 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 case OP_SUBL:
 if (s1->prefix & PREFIX_LOCK) {
 tcg_gen_neg_tl(cpu_T0, cpu_T1);
-tcg_gen_atomic_fetch_add_tl(cpu_cc_srcT, cpu_A0, cpu_T0,
+tcg_gen_atomic_fetch_add_tl(s1->cc_srcT, cpu_A0, cpu_T0,
 s1->mem_index, ot | MO_LE);
-tcg_gen_sub_tl(cpu_T0, cpu_cc_srcT, cpu_T1);
+tcg_gen_sub_tl(cpu_T0, s1->cc_srcT, cpu_T1);
 } else {
-tcg_gen_mov_tl(cpu_cc_srcT, cpu_T0);
+tcg_gen_mov_tl(s1->cc_srcT, cpu_T0);
 tcg_gen_sub_tl(cpu_T0, cpu_T0, cpu_T1);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
@@ -1356,7 +1360,7 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp ot, 
int d)
 break;
 case OP_CMPL:
 tcg_gen_mov_tl(cpu_cc_src, cpu_T1);
-tcg_gen_mov_tl(cpu_cc_srcT, cpu_T0);
+tcg_gen_mov_tl(s1->cc_srcT, cpu_T0);
 tcg_gen_sub_tl(cpu_cc_dst, cpu_T0, cpu_T1);
 set_cc_op(s1, CC_OP_SUBB + ot);
 break;
@@ -4823,7 +4827,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState 
*cpu)
 gen_op_mov_reg_v(ot, rm, cpu_T0);
 }
 }
-gen_op_update_neg_cc();
+gen_op_update_neg_cc(s);
 set_cc_op(s, CC_OP_SUBB + ot);
 break;
 case 4: /* mul */
@@ -5283,7 +5287,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState 
*cpu)
 }
 }
 tcg_gen_mov_tl(cpu_cc_src, oldv);
-tcg_gen_mov_tl(cpu_cc_srcT, cmpv);
+tcg_gen_mov_tl(s->cc_srcT, cmpv);
 tcg_gen_sub_tl(cpu_cc_dst, cmpv, oldv);
 set_cc_op(s, CC_OP_SUBB + ot);
 tcg_temp_free(oldv);
@@ -8463,7 +8467,7 @@ static void i386_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cpu)
 cpu_tmp4 = tcg_temp_new();
 cpu_ptr0 = tcg_temp_new_ptr();
 cpu_ptr1 = tcg_temp_new_ptr();
-cpu_cc_srcT = tcg_temp_local_new();
+dc->cc_srcT = tcg_temp_local_new();
 }
 
 static void i386_tr_tb_start(DisasContextBase

[Qemu-devel] [PATCH v3 00/13] i386 + x86_64 mttcg

v2: https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01122.html

Changes since v2:

- Add rth's R-b tag to the last patch
- Drop v2's first 10 patches, since Paolo already picked those up
- Move TCG temps + x86_64_hregs to DisasContext
  + While at it, drop the cpu_ prefix from the TCG temps,
e.g. cpu_A0 -> s->A0
  + Split the conversion into separate patches to ease review.
The patches are quite boring and long because the temps
are everywhere, and I had to add DisasContext *s to quite a few
functions

The series is checkpatch-clean.

You can fetch these patches from:
  https://github.com/cota/qemu/tree/i386-mttcg-v3

Thanks,

Emilio

[Qemu-devel] [PATCH v3 10/13] target/i386: move cpu_tmp3_i32 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 64 -
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index ec68f7dba1..cd880cc2a8 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -79,7 +79,6 @@ static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
 
-static TCGv_i32 cpu_tmp3_i32;
 static TCGv_i64 cpu_tmp1_i64;
 
 #include "exec/gen-icount.h"
@@ -143,6 +142,7 @@ typedef struct DisasContext {
 TCGv_ptr ptr0;
 TCGv_ptr ptr1;
 TCGv_i32 tmp2_i32;
+TCGv_i32 tmp3_i32;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -1159,8 +1159,8 @@ static inline void gen_outs(DisasContext *s, TCGMemOp ot)
 
 tcg_gen_trunc_tl_i32(s->tmp2_i32, cpu_regs[R_EDX]);
 tcg_gen_andi_i32(s->tmp2_i32, s->tmp2_i32, 0x);
-tcg_gen_trunc_tl_i32(cpu_tmp3_i32, s->T0);
-gen_helper_out_func(ot, s->tmp2_i32, cpu_tmp3_i32);
+tcg_gen_trunc_tl_i32(s->tmp3_i32, s->T0);
+gen_helper_out_func(ot, s->tmp2_i32, s->tmp3_i32);
 gen_op_movl_T0_Dshift(s, ot);
 gen_op_add_reg_T0(s, s->aflag, R_ESI);
 gen_bpt_io(s, s->tmp2_i32, ot);
@@ -1426,8 +1426,8 @@ static void gen_shift_flags(DisasContext *s, TCGMemOp ot, 
TCGv result,
 if (s->cc_op == CC_OP_DYNAMIC) {
 oldop = cpu_cc_op;
 } else {
-tcg_gen_movi_i32(cpu_tmp3_i32, s->cc_op);
-oldop = cpu_tmp3_i32;
+tcg_gen_movi_i32(s->tmp3_i32, s->cc_op);
+oldop = s->tmp3_i32;
 }
 
 /* Conditionally store the CC_OP value.  */
@@ -1546,11 +1546,11 @@ static void gen_rot_rm_T1(DisasContext *s, TCGMemOp ot, 
int op1, int is_right)
 #ifdef TARGET_X86_64
 case MO_32:
 tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
-tcg_gen_trunc_tl_i32(cpu_tmp3_i32, s->T1);
+tcg_gen_trunc_tl_i32(s->tmp3_i32, s->T1);
 if (is_right) {
-tcg_gen_rotr_i32(s->tmp2_i32, s->tmp2_i32, cpu_tmp3_i32);
+tcg_gen_rotr_i32(s->tmp2_i32, s->tmp2_i32, s->tmp3_i32);
 } else {
-tcg_gen_rotl_i32(s->tmp2_i32, s->tmp2_i32, cpu_tmp3_i32);
+tcg_gen_rotl_i32(s->tmp2_i32, s->tmp2_i32, s->tmp3_i32);
 }
 tcg_gen_extu_i32_tl(s->T0, s->tmp2_i32);
 break;
@@ -1593,9 +1593,9 @@ static void gen_rot_rm_T1(DisasContext *s, TCGMemOp ot, 
int op1, int is_right)
 t1 = tcg_temp_new_i32();
 tcg_gen_trunc_tl_i32(t1, s->T1);
 tcg_gen_movi_i32(s->tmp2_i32, CC_OP_ADCOX);
-tcg_gen_movi_i32(cpu_tmp3_i32, CC_OP_EFLAGS);
+tcg_gen_movi_i32(s->tmp3_i32, CC_OP_EFLAGS);
 tcg_gen_movcond_i32(TCG_COND_NE, cpu_cc_op, t1, t0,
-s->tmp2_i32, cpu_tmp3_i32);
+s->tmp2_i32, s->tmp3_i32);
 tcg_temp_free_i32(t0);
 tcg_temp_free_i32(t1);
 
@@ -3912,11 +3912,11 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 switch (ot) {
 default:
 tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
-tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_regs[R_EDX]);
-tcg_gen_mulu2_i32(s->tmp2_i32, cpu_tmp3_i32,
-  s->tmp2_i32, cpu_tmp3_i32);
+tcg_gen_trunc_tl_i32(s->tmp3_i32, cpu_regs[R_EDX]);
+tcg_gen_mulu2_i32(s->tmp2_i32, s->tmp3_i32,
+  s->tmp2_i32, s->tmp3_i32);
 tcg_gen_extu_i32_tl(cpu_regs[s->vex_v], s->tmp2_i32);
-tcg_gen_extu_i32_tl(cpu_regs[reg], cpu_tmp3_i32);
+tcg_gen_extu_i32_tl(cpu_regs[reg], s->tmp3_i32);
 break;
 #ifdef TARGET_X86_64
 case MO_64:
@@ -4882,11 +4882,11 @@ static target_ulong disas_insn(DisasContext *s, 
CPUState *cpu)
 default:
 case MO_32:
 tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
-tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_regs[R_EAX]);
-tcg_gen_mulu2_i32(s->tmp2_i32, cpu_tmp3_i32,
-  s->tmp2_i32, cpu_tmp3_i32);
+tcg_gen_trunc_tl_i32(s->tmp3_i32, cpu_regs[R_EAX]);
+tcg_gen_mulu2_i32(s->tmp2_i32, s->tmp3_i32,
+  s->tmp2_i32, s->tmp3_i32);
 tcg_gen_extu_i32_tl(cpu_regs[R_EAX], s->tmp2_i32);
-tcg_gen_extu_i32_tl(cpu_regs[R_EDX], cpu_tmp3_i32);
+tcg_gen_extu_i32_tl(cpu_regs[R_EDX], s->tmp3_i32);
 tcg_gen_mov_tl(cpu_cc_dst, cpu_regs[R_EAX]);
 tcg_gen_mov_tl(cpu_cc_src, cpu_regs[R_EDX]);
 set_cc_op(s, CC_OP_MULL);
@@ -4933,14 +4933,14 @@ static target_ulong disas_insn(DisasContext *s, 
CPUState *cpu)
 default:
 case MO_32:
 tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
-tcg_gen_trunc_tl_i32(cpu_tmp3_i32, cpu_regs[R_EAX]);
-

[Qemu-devel] [PATCH v3 04/13] target/i386: move cpu_T1 to DisasContext

Signed-off-by: Emilio G. Cota 
---
 target/i386/translate.c | 341 
 1 file changed, 170 insertions(+), 171 deletions(-)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 73fd7e5b9a..bd27e65344 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -78,8 +78,6 @@ static TCGv cpu_regs[CPU_NB_REGS];
 static TCGv cpu_seg_base[6];
 static TCGv_i64 cpu_bndl[4];
 static TCGv_i64 cpu_bndu[4];
-/* local temps */
-static TCGv cpu_T1;
 /* local register indexes (only used inside old micro ops) */
 static TCGv cpu_tmp0, cpu_tmp4;
 static TCGv_ptr cpu_ptr0, cpu_ptr1;
@@ -139,6 +137,7 @@ typedef struct DisasContext {
 TCGv cc_srcT;
 TCGv A0;
 TCGv T0;
+TCGv T1;
 
 sigjmp_buf jmpbuf;
 } DisasContext;
@@ -656,20 +655,20 @@ static void gen_op_update1_cc(DisasContext *s)
 
 static void gen_op_update2_cc(DisasContext *s)
 {
-tcg_gen_mov_tl(cpu_cc_src, cpu_T1);
+tcg_gen_mov_tl(cpu_cc_src, s->T1);
 tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 }
 
 static void gen_op_update3_cc(DisasContext *s, TCGv reg)
 {
 tcg_gen_mov_tl(cpu_cc_src2, reg);
-tcg_gen_mov_tl(cpu_cc_src, cpu_T1);
+tcg_gen_mov_tl(cpu_cc_src, s->T1);
 tcg_gen_mov_tl(cpu_cc_dst, s->T0);
 }
 
 static inline void gen_op_testl_T0_T1_cc(DisasContext *s)
 {
-tcg_gen_and_tl(cpu_cc_dst, s->T0, cpu_T1);
+tcg_gen_and_tl(cpu_cc_dst, s->T0, s->T1);
 }
 
 static void gen_op_update_neg_cc(DisasContext *s)
@@ -1090,7 +1089,7 @@ static inline void gen_lods(DisasContext *s, TCGMemOp ot)
 static inline void gen_scas(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_EDI(s);
-gen_op_ld_v(s, ot, cpu_T1, s->A0);
+gen_op_ld_v(s, ot, s->T1, s->A0);
 gen_op(s, OP_CMPL, ot, R_EAX);
 gen_op_movl_T0_Dshift(s, ot);
 gen_op_add_reg_T0(s, s->aflag, R_EDI);
@@ -1099,7 +1098,7 @@ static inline void gen_scas(DisasContext *s, TCGMemOp ot)
 static inline void gen_cmps(DisasContext *s, TCGMemOp ot)
 {
 gen_string_movl_A0_EDI(s);
-gen_op_ld_v(s, ot, cpu_T1, s->A0);
+gen_op_ld_v(s, ot, s->T1, s->A0);
 gen_string_movl_A0_ESI(s);
 gen_op(s, OP_CMPL, ot, OR_TMP0);
 gen_op_movl_T0_Dshift(s, ot);
@@ -1274,11 +1273,11 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 case OP_ADCL:
 gen_compute_eflags_c(s1, cpu_tmp4);
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_add_tl(s1->T0, cpu_tmp4, cpu_T1);
+tcg_gen_add_tl(s1->T0, cpu_tmp4, s1->T1);
 tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
 s1->mem_index, ot | MO_LE);
 } else {
-tcg_gen_add_tl(s1->T0, s1->T0, cpu_T1);
+tcg_gen_add_tl(s1->T0, s1->T0, s1->T1);
 tcg_gen_add_tl(s1->T0, s1->T0, cpu_tmp4);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
@@ -1288,12 +1287,12 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 case OP_SBBL:
 gen_compute_eflags_c(s1, cpu_tmp4);
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_add_tl(s1->T0, cpu_T1, cpu_tmp4);
+tcg_gen_add_tl(s1->T0, s1->T1, cpu_tmp4);
 tcg_gen_neg_tl(s1->T0, s1->T0);
 tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
 s1->mem_index, ot | MO_LE);
 } else {
-tcg_gen_sub_tl(s1->T0, s1->T0, cpu_T1);
+tcg_gen_sub_tl(s1->T0, s1->T0, s1->T1);
 tcg_gen_sub_tl(s1->T0, s1->T0, cpu_tmp4);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
@@ -1302,10 +1301,10 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 break;
 case OP_ADDL:
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, cpu_T1,
+tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T1,
 s1->mem_index, ot | MO_LE);
 } else {
-tcg_gen_add_tl(s1->T0, s1->T0, cpu_T1);
+tcg_gen_add_tl(s1->T0, s1->T0, s1->T1);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
 gen_op_update2_cc(s1);
@@ -1313,13 +1312,13 @@ static void gen_op(DisasContext *s1, int op, TCGMemOp 
ot, int d)
 break;
 case OP_SUBL:
 if (s1->prefix & PREFIX_LOCK) {
-tcg_gen_neg_tl(s1->T0, cpu_T1);
+tcg_gen_neg_tl(s1->T0, s1->T1);
 tcg_gen_atomic_fetch_add_tl(s1->cc_srcT, s1->A0, s1->T0,
 s1->mem_index, ot | MO_LE);
-tcg_gen_sub_tl(s1->T0, s1->cc_srcT, cpu_T1);
+tcg_gen_sub_tl(s1->T0, s1->cc_srcT, s1->T1);
 } else {
 tcg_gen_mov_tl(s1->cc_srcT, s1->T0);
-tcg_gen_sub_tl(s1->T0, s1->T0, cpu_T1);
+tcg_gen_sub_tl(s1->T0, s1->T0, s1->T1);
 gen_op_st_rm_T0_A0(s1, ot, d);
 }
 gen_op_update2_cc(s1);
@@ -1328,10 +1327,10 @@ static void gen_op(DisasContext *

Re: [Qemu-devel] [PATCH v2 4/4] qapi: add transaction support for x-block-dirty-bitmap-merge




On 07/06/2018 07:36 AM, Vladimir Sementsov-Ogievskiy wrote:
> New action is like clean action: do the whole thing in .prepare and
> undo in .abort. This behavior for bitmap-changing actions is needed
> because backup job actions use bitmap in .prepare.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/transaction.json |  2 ++
>  blockdev.c| 38 +-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index d7e4274550..5875cdb16c 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -48,6 +48,7 @@
>  # - @block-dirty-bitmap-clear: since 2.5
>  # - @x-block-dirty-bitmap-enable: since 3.0
>  # - @x-block-dirty-bitmap-disable: since 3.0
> +# - @x-block-dirty-bitmap-merge: since 3.1
>  # - @blockdev-backup: since 2.3
>  # - @blockdev-snapshot: since 2.5
>  # - @blockdev-snapshot-internal-sync: since 1.7
> @@ -63,6 +64,7 @@
> 'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
> 'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
> 'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
> +   'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
> 'blockdev-backup': 'BlockdevBackup',
> 'blockdev-snapshot': 'BlockdevSnapshot',
> 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
> diff --git a/blockdev.c b/blockdev.c
> index 5348e8ba9b..feebbb9a9a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1940,6 +1940,7 @@ static void blockdev_backup_clean(BlkActionState 
> *common)
>  typedef struct BlockDirtyBitmapState {
>  BlkActionState common;
>  BdrvDirtyBitmap *bitmap;
> +BdrvDirtyBitmap *merge_source;

Is this necessary?

>  BlockDriverState *bs;
>  HBitmap *backup;
>  bool prepared;
> @@ -2112,6 +2113,35 @@ static void 
> block_dirty_bitmap_disable_abort(BlkActionState *common)
>  }
>  }
>  
> +static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
> + Error **errp)
> +{
> +BlockDirtyBitmapMerge *action;
> +BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> + common, common);
> +
> +if (action_check_completion_mode(common, errp) < 0) {
> +return;
> +}
> +
> +action = common->action->u.x_block_dirty_bitmap_merge.data;
> +state->bitmap = block_dirty_bitmap_lookup(action->node,
> +  action->dst_name,
> +  &state->bs,
> +  errp);
> +if (!state->bitmap) {
> +return;
> +}
> +
> +state->merge_source = bdrv_find_dirty_bitmap(state->bs, 
> action->src_name);
> +if (!state->merge_source) {
> +return;
> +}
> +
> +bdrv_merge_dirty_bitmap(state->bitmap, state->merge_source, 
> &state->backup,
> +errp);
> +}
> +
>  static void abort_prepare(BlkActionState *common, Error **errp)
>  {
>  error_setg(errp, "Transaction aborted using Abort action");
> @@ -2182,7 +2212,13 @@ static const BlkActionOps actions[] = {
>  .instance_size = sizeof(BlockDirtyBitmapState),
>  .prepare = block_dirty_bitmap_disable_prepare,
>  .abort = block_dirty_bitmap_disable_abort,
> - }
> +},
> +[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
> +.instance_size = sizeof(BlockDirtyBitmapState),
> +.prepare = block_dirty_bitmap_merge_prepare,
> +.commit = block_dirty_bitmap_free_backup,
> +.abort = block_dirty_bitmap_restore,
> +}
>  };
>  
>  /**
> 

If the new state is not necessary and you remove it:

Reviewed-by: John Snow

[Qemu-devel] [Bug 1788665] Re: Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM using "Spectre" protection

2018-09-11 Thread George Amanakis

Hello All,

I can reproduce this on two different systems with Ivy-Bridge CPUs:
Xeon E5 2667v2 / X9SRA running Fedora 28, with Windows 10 1803 as KVM guest
Xeon E3 1270v2 / X9SCM running Archlinux, with Windows 10 1803 as KVM guest

The performance degradation doesn't occur when the Windows 10 guest
disables the spectre mitigation using "InSpectre.exe", or when the spec-
ctrl flag is disabled in libvirt, or when the cpu-microcode isn't
updated in the host.

I followed the latest suggestion, and enabled "--enable-debug" in
qemu-3.0 and also compiled kernel-4.17.19 with CONFIG_DEBUG_INFO=y.
However I cannot see any differences while running "perf top" in the
host between the affected/unaffected version of windows. CPU usage seems
to be the same.

Any hints would be greatly appreciated.

Best,
George

--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1788665

Title:
Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM
using "Spectre" protection

Status in QEMU:
New

Bug description:
Windows 10 (1803) VM using VGA passthrough via qemu script.

After upgrading Windows 10 Pro VM to version 1803, or possibly after
applying the March/April security updates from Microsoft, the VM would
show low 2D graphics performance (sluggishness in 2D applications and
low Passmark results).

Turning off Spectre vulnerability protection in Windows remedies the
issue.

Expected behavior:
qemu/kvm hypervisor to expose firmware capabilities of host to guest OS - see
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/CVE-2017-5715-and-hyper-v-vms

Background:

Starting in March or April Microsoft began to push driver updates in
their updates / security updates. See https://support.microsoft.com
/en-us/help/4073757/protect-your-windows-devices-against-spectre-
meltdown

One update concerns the Intel microcode - see
https://support.microsoft.com/en-us/help/4100347. It is activated by
default within Windows.

Once the updates are applied within the Windows guest, 2D graphics
performance drops significantly. Other performance benchmarks are not
affected.

A bare metal Windows installation does not display a performance loss
after the update. See https://heiko-sieger.info/low-2d-graphics-
benchmark-with-windows-10-1803-kvm-vm/

Similar reports can be found here:

https://www.reddit.com/r/VFIO/comments/97unx4/passmark_lousy_2d_graphics_performance_on_windows/

Hardware:

6 core Intel Core i7-3930K (-MT-MCP-)

Host OS:
Linux Mint 19/Ubuntu 18.04
Kernel: 4.15.0-32-generic x86_64
Qemu: QEMU emulator version 2.11.1
Intel microcode (host): 0x714
dmesg | grep microcode
[0.00] microcode: microcode updated early to revision 0x714, date =
2018-05-08
[2.810683] microcode: sig=0x206d7, pf=0x4, revision=0x714
[2.813340] microcode: Microcode Update Driver: v2.2.

Note: I manually updated the Intel microcode on the host from 0x713 to
0x714. However, both microcode versions produce the same result in the
Windows guest.

Guest OS:
Windows 10 Pro 64 bit, release 1803

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1788665/+subscriptions

Re: [Qemu-devel] [Bug 1791796] Re: unimplemented thread syscalls in nios2 user-mode emulation



Sandra Loosemore <1791...@bugs.launchpad.net> writes:

> If you need a Nios II GNU/Linux toolchain, I think the most recent
> CodeBench Lite release will work:
>
> https://sourcery.mentor.com/GNUToolchain/subscription42545

Hmm I tried automating that but it seems the installer has GTK
dependencies!?

  Setup:
  GTK+ Version Check
  Setup:
  An error has occurred. See the log file
  /root/.mentor/logs/20180911185933/.metadata/.log.
  root@0ef91b5e50f2:/opt# cat /root/.mentor/logs/20180911185933/.metadata/.log
  !SESSION 2018-09-11 18:59:36.197 
---
  eclipse.buildId=unknown
  java.version=1.8.0_102
  java.vendor=Oracle Corporation
  BootLoader constants: OS=linux, ARCH=x86, WS=gtk, NL=en_US
  Framework arguments:  -install.once -install.data=/root/.mentor
  Command-line arguments:  -os linux -ws gtk -arch x86 -install.once 
-install.data=/root/.mentor -data /root/.mentor/logs/20180911185933

  !ENTRY org.eclipse.osgi 4 0 2018-09-11 18:59:37.407
  !MESSAGE Application error
  !STACK 1
  java.lang.UnsatisfiedLinkError: Could not load SWT library. Reasons:
  
/tmp/sourceryg++-2018.05-5-nios2-linux-gnu.bin_sfx.f9eaefb7/installer/configuration/org.eclipse.osgi/148/0/.cp/libswt-pi-gtk-4530.so:
 libgtk-x11-2.0.so.0: cannot open shared object file: No such file or directory
  no swt-pi-gtk in java.library.path
  Can't load library: /root/.swt/lib/linux/x86/libswt-pi-gtk-4530.so
  Can't load library: /root/.swt/lib/linux/x86/libswt-pi-gtk.so
  /root/.swt/lib/linux/x86/libswt-pi-gtk-4530.so: libgtk-x11-2.0.so.0: 
cannot open shared object file: No such file or directory

Should I just try the tarball approach?

> We're planning on adding user-mode QEMU to the upcoming 2018.11
> release  that's actually what I've been testing it for.  Results on
> the GCC testsuite actually don't look too bad,

OK - I'm a little surprised given the failures I saw in our own
test/tcg/multiarch but it's totally possible that:

  - the buildroot toolchain if foobar
  - the (ancient) tests need tweaking

but it would be nice if we get to a point that QEMU's internal
linux-user tests also pass.

> but I have a patch I
> haven't submitted yet that's required to make the GDB stub work, and
> there are a lot of GDB test failures I haven't triaged yet.

When you do post the gdb patch feel free to CC me as I've poked around
in that before.

--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames




On 09/11/2018 02:37 PM, Jeff Cody wrote:
> On Tue, Sep 11, 2018 at 02:22:31PM -0400, John Snow wrote:
>>
>>
>> On 09/11/2018 01:15 AM, Jeff Cody wrote:
>>> When we converted rbd to get rid of the older key/value-centric
>>> encoding format, we broke compatibility with image files with backing
>>> file strings encoded in the old format.
>>>
>>> This leaves a bit of an ugly conundrum, and a hacky solution.
>>>
>>> If the initial attempt to parse the "proper" options fails, it assumes
>>> that we may have an older key/value encoded filename.  Fall back to
>>> attempting to parse the filename, and extract the required options from
>>> it.  If that fails, pass along the original error message.
>>>
>>> This approach has a potential drawback: if for some reason there are
>>> some options supplied the new way, and some the old way, we may not
>>> catch all the old options if they are not required options (since it
>>> won't cause the initial failure).
>>>
>>> Signed-off-by: Jeff Cody 
>>> ---
>>>  block/rbd.c | 30 +++---
>>>  1 file changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block/rbd.c b/block/rbd.c
>>> index a8e79d01d2..bce86b8bde 100644
>>> --- a/block/rbd.c
>>> +++ b/block/rbd.c
>>> @@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
>>> *options, int flags,
>>>  BlockdevOptionsRbd *opts = NULL;
>>>  const QDictEntry *e;
>>>  Error *local_err = NULL;
>>> -char *keypairs, *secretid;
>>> +char *keypairs, *secretid, *filename;
>>>  int r;
>>>  
>>>  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
>>> @@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
>>> *options, int flags,
>>>  
>>>  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>>>  if (local_err) {
>>> -error_propagate(errp, local_err);
>>> -goto out;
>>> +/* If the initial attempt to convert and process the options 
>>> failed,
>>> + * we may be attempting to open an image file that has the rbd 
>>> options
>>> + * specified in the older format consisting of all key/value pairs
>>> + * encoded in the filename.  Go ahead and attempt to parse the
>>> + * filename, and see if we can pull out the required options */
>>
>> Is it worth splitting out the legacy parsing routine here into its own
>> function, given that we will generally depend on it not being invoked?
>> i.e., for readability, it doesn't need to distract us.
>>
> 
> Yeah, that would probably be good.
> 
>>> +Error *parse_err = NULL;
>>> +
>>> +filename = g_strdup(qdict_get_try_str(options, "filename"));
>>> +qdict_del(options, "filename");
>>> +
>>> +qemu_rbd_parse_filename(filename, options, NULL);
>>> +
>>> +g_free(keypairs);
>>
>> As Eric already noticed, better to just return with error if this is
>> already set.
>>
>>> +keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
>>> +if (keypairs) {
>>> +qdict_del(options, "=keyvalue-pairs");
>>> +}
>>> +
>>> +r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
>>> +if (parse_err) {
>>> +/* if the second attempt failed, pass along the original error
>>> + * message for the current format */
>>> +error_propagate(errp, local_err);
>>> +error_free(parse_err);
>>> +goto out;
>>> +}
>>
>> If it does succeed, though, ought we emit a deprecated warning that the
>> old specification syntax is not supported?
>>
> 
> I don't know.  Without this support, we can't open some existing images.  At
> what point would we actually remove that support?
> 

Yeah, probably never -- but there are some versions of QEMU in the wild
that now simply don't support this format, so some urging to switch is
probably not harmful was my thought.

(At least, that was my read. When did we switch RBD formats? Just for
3.1? In that case, no warning is necessary.)

>> Once we load the image, will the header get rewritten into a compliant
>> format?
>>
> 
> Hmm - I think in some code paths, but not all.  I don't think the answer is
> 'yes' universally, alas.
>

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames

On Tue, Sep 11, 2018 at 02:22:31PM -0400, John Snow wrote:
> 
> 
> On 09/11/2018 01:15 AM, Jeff Cody wrote:
> > When we converted rbd to get rid of the older key/value-centric
> > encoding format, we broke compatibility with image files with backing
> > file strings encoded in the old format.
> > 
> > This leaves a bit of an ugly conundrum, and a hacky solution.
> > 
> > If the initial attempt to parse the "proper" options fails, it assumes
> > that we may have an older key/value encoded filename.  Fall back to
> > attempting to parse the filename, and extract the required options from
> > it.  If that fails, pass along the original error message.
> > 
> > This approach has a potential drawback: if for some reason there are
> > some options supplied the new way, and some the old way, we may not
> > catch all the old options if they are not required options (since it
> > won't cause the initial failure).
> > 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block/rbd.c | 30 +++---
> >  1 file changed, 27 insertions(+), 3 deletions(-)
> > 
> > diff --git a/block/rbd.c b/block/rbd.c
> > index a8e79d01d2..bce86b8bde 100644
> > --- a/block/rbd.c
> > +++ b/block/rbd.c
> > @@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >  BlockdevOptionsRbd *opts = NULL;
> >  const QDictEntry *e;
> >  Error *local_err = NULL;
> > -char *keypairs, *secretid;
> > +char *keypairs, *secretid, *filename;
> >  int r;
> >  
> >  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> > @@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >  
> >  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
> >  if (local_err) {
> > -error_propagate(errp, local_err);
> > -goto out;
> > +/* If the initial attempt to convert and process the options 
> > failed,
> > + * we may be attempting to open an image file that has the rbd 
> > options
> > + * specified in the older format consisting of all key/value pairs
> > + * encoded in the filename.  Go ahead and attempt to parse the
> > + * filename, and see if we can pull out the required options */
> 
> Is it worth splitting out the legacy parsing routine here into its own
> function, given that we will generally depend on it not being invoked?
> i.e., for readability, it doesn't need to distract us.
> 

Yeah, that would probably be good.

> > +Error *parse_err = NULL;
> > +
> > +filename = g_strdup(qdict_get_try_str(options, "filename"));
> > +qdict_del(options, "filename");
> > +
> > +qemu_rbd_parse_filename(filename, options, NULL);
> > +
> > +g_free(keypairs);
> 
> As Eric already noticed, better to just return with error if this is
> already set.
> 
> > +keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> > +if (keypairs) {
> > +qdict_del(options, "=keyvalue-pairs");
> > +}
> > +
> > +r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
> > +if (parse_err) {
> > +/* if the second attempt failed, pass along the original error
> > + * message for the current format */
> > +error_propagate(errp, local_err);
> > +error_free(parse_err);
> > +goto out;
> > +}
> 
> If it does succeed, though, ought we emit a deprecated warning that the
> old specification syntax is not supported?
>

I don't know.  Without this support, we can't open some existing images.  At
what point would we actually remove that support?

> Once we load the image, will the header get rewritten into a compliant
> format?
> 

Hmm - I think in some code paths, but not all.  I don't think the answer is
'yes' universally, alas.

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames

On Tue, Sep 11, 2018 at 01:03:44PM -0500, Eric Blake wrote:
> On 9/11/18 12:15 AM, Jeff Cody wrote:
> >When we converted rbd to get rid of the older key/value-centric
> >encoding format, we broke compatibility with image files with backing
> >file strings encoded in the old format.
> >
> >This leaves a bit of an ugly conundrum, and a hacky solution.
> >
> >If the initial attempt to parse the "proper" options fails, it assumes
> >that we may have an older key/value encoded filename.  Fall back to
> >attempting to parse the filename, and extract the required options from
> >it.  If that fails, pass along the original error message.
> >
> >This approach has a potential drawback: if for some reason there are
> >some options supplied the new way, and some the old way, we may not
> >catch all the old options if they are not required options (since it
> >won't cause the initial failure).
> 
> No one should be mixing new and old, though.
> 
> >
> >Signed-off-by: Jeff Cody 
> >---
> >  block/rbd.c | 30 +++---
> >  1 file changed, 27 insertions(+), 3 deletions(-)
> >
> >diff --git a/block/rbd.c b/block/rbd.c
> >index a8e79d01d2..bce86b8bde 100644
> >--- a/block/rbd.c
> >+++ b/block/rbd.c
> >@@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> >*options, int flags,
> >  BlockdevOptionsRbd *opts = NULL;
> >  const QDictEntry *e;
> >  Error *local_err = NULL;
> >-char *keypairs, *secretid;
> >+char *keypairs, *secretid, *filename;
> >  int r;
> >  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> >@@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> >*options, int flags,
> >  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
> >  if (local_err) {
> >-error_propagate(errp, local_err);
> >-goto out;
> 
> Oh, my comment about simplifying this in 1/2 is probably moot, now that you
> are doing a lot more based on local_err rather than just blindly propagating
> it.
> 
> >+/* If the initial attempt to convert and process the options failed,
> >+ * we may be attempting to open an image file that has the rbd 
> >options
> >+ * specified in the older format consisting of all key/value pairs
> >+ * encoded in the filename.  Go ahead and attempt to parse the
> >+ * filename, and see if we can pull out the required options */
> >+Error *parse_err = NULL;
> >+
> >+filename = g_strdup(qdict_get_try_str(options, "filename"));
> 
> You already spotted your leak.
> 
> >+qdict_del(options, "filename");
> >+
> >+qemu_rbd_parse_filename(filename, options, NULL);
> >+
> >+g_free(keypairs);
> 
> Wait. Why are you unilaterally freeing any previously-parsed keypairs in
> favor of the ones parsed out of the filename?  I'd rather that we insist on
> only old behavior, or only new, and not some mix.  Thus, if we already
> detected keypairs at all, we should declare this situation as an error,
> rather than throwing them away.
> 

Good point.  I'll flag (local_err && keypairs) as an error.

I also just realized I need to check for NULL filename, and error out as
well in that case.

> >+keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> >+if (keypairs) {
> >+qdict_del(options, "=keyvalue-pairs");
> >+}
> >+
> >+r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
> >+if (parse_err) {
> >+/* if the second attempt failed, pass along the original error
> >+ * message for the current format */
> >+error_propagate(errp, local_err);
> >+error_free(parse_err);
> >+goto out;
> >+}
> >  }
> 
> The idea of trying two parses makes sense, but I'm hoping v2 better handles
> the case of detecting bad attempts to mix-and-match behavior. Furthermore,
> is there an iotests that you can modify (or add) as a regression test for
> this working the way we want?

Hmm... yes, that should be doable.  I'm not sure if I can do it without the
'success' path being a timeout when there is no ceph server present, though.
I'll see what I can do.

-Jeff

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames




On 09/11/2018 01:15 AM, Jeff Cody wrote:
> When we converted rbd to get rid of the older key/value-centric
> encoding format, we broke compatibility with image files with backing
> file strings encoded in the old format.
> 
> This leaves a bit of an ugly conundrum, and a hacky solution.
> 
> If the initial attempt to parse the "proper" options fails, it assumes
> that we may have an older key/value encoded filename.  Fall back to
> attempting to parse the filename, and extract the required options from
> it.  If that fails, pass along the original error message.
> 
> This approach has a potential drawback: if for some reason there are
> some options supplied the new way, and some the old way, we may not
> catch all the old options if they are not required options (since it
> won't cause the initial failure).
> 
> Signed-off-by: Jeff Cody 
> ---
>  block/rbd.c | 30 +++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index a8e79d01d2..bce86b8bde 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  BlockdevOptionsRbd *opts = NULL;
>  const QDictEntry *e;
>  Error *local_err = NULL;
> -char *keypairs, *secretid;
> +char *keypairs, *secretid, *filename;
>  int r;
>  
>  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> @@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>  if (local_err) {
> -error_propagate(errp, local_err);
> -goto out;
> +/* If the initial attempt to convert and process the options failed,
> + * we may be attempting to open an image file that has the rbd 
> options
> + * specified in the older format consisting of all key/value pairs
> + * encoded in the filename.  Go ahead and attempt to parse the
> + * filename, and see if we can pull out the required options */

Is it worth splitting out the legacy parsing routine here into its own
function, given that we will generally depend on it not being invoked?
i.e., for readability, it doesn't need to distract us.

> +Error *parse_err = NULL;
> +
> +filename = g_strdup(qdict_get_try_str(options, "filename"));
> +qdict_del(options, "filename");
> +
> +qemu_rbd_parse_filename(filename, options, NULL);
> +
> +g_free(keypairs);

As Eric already noticed, better to just return with error if this is
already set.

> +keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> +if (keypairs) {
> +qdict_del(options, "=keyvalue-pairs");
> +}
> +
> +r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
> +if (parse_err) {
> +/* if the second attempt failed, pass along the original error
> + * message for the current format */
> +error_propagate(errp, local_err);
> +error_free(parse_err);
> +goto out;
> +}

If it does succeed, though, ought we emit a deprecated warning that the
old specification syntax is not supported?

Once we load the image, will the header get rewritten into a compliant
format?

--js

>  }
>  
>  /* Remove the processed options from the QDict (the visitor processes
>

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames


On 9/11/18 12:15 AM, Jeff Cody wrote:

When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.

This leaves a bit of an ugly conundrum, and a hacky solution.

If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename.  Fall back to
attempting to parse the filename, and extract the required options from
it.  If that fails, pass along the original error message.

This approach has a potential drawback: if for some reason there are
some options supplied the new way, and some the old way, we may not
catch all the old options if they are not required options (since it
won't cause the initial failure).


No one should be mixing new and old, though.



Signed-off-by: Jeff Cody 
---
  block/rbd.c | 30 +++---
  1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index a8e79d01d2..bce86b8bde 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
  BlockdevOptionsRbd *opts = NULL;
  const QDictEntry *e;
  Error *local_err = NULL;
-char *keypairs, *secretid;
+char *keypairs, *secretid, *filename;
  int r;
  
  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));

@@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
  
  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);

  if (local_err) {
-error_propagate(errp, local_err);
-goto out;


Oh, my comment about simplifying this in 1/2 is probably moot, now that 
you are doing a lot more based on local_err rather than just blindly 
propagating it.



+/* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename.  Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options */
+Error *parse_err = NULL;
+
+filename = g_strdup(qdict_get_try_str(options, "filename"));


You already spotted your leak.


+qdict_del(options, "filename");
+
+qemu_rbd_parse_filename(filename, options, NULL);
+
+g_free(keypairs);


Wait. Why are you unilaterally freeing any previously-parsed keypairs in 
favor of the ones parsed out of the filename?  I'd rather that we insist 
on only old behavior, or only new, and not some mix.  Thus, if we 
already detected keypairs at all, we should declare this situation as an 
error, rather than throwing them away.



+keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+if (keypairs) {
+qdict_del(options, "=keyvalue-pairs");
+}
+
+r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
+if (parse_err) {
+/* if the second attempt failed, pass along the original error
+ * message for the current format */
+error_propagate(errp, local_err);
+error_free(parse_err);
+goto out;
+}
  }


The idea of trying two parses makes sense, but I'm hoping v2 better 
handles the case of detecting bad attempts to mix-and-match behavior. 
Furthermore, is there an iotests that you can modify (or add) as a 
regression test for this working the way we want?


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] block/rbd: pull out qemu_rbd_convert_options




On 09/11/2018 01:15 AM, Jeff Cody wrote:
> Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
> into a helper function.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block/rbd.c | 36 
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index ca8e5bbace..a8e79d01d2 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -655,12 +655,34 @@ failed_opts:
>  return r;
>  }
>  
> +static int qemu_rbd_convert_options(BlockDriverState *bs, QDict *options,
> +BlockdevOptionsRbd **opts, Error **errp)
> +{
> +Visitor *v;
> +Error *local_err = NULL;
> +
> +/* Convert the remaining options into a QAPI object */
> +v = qobject_input_visitor_new_flat_confused(options, errp);
> +if (!v) {
> +return -EINVAL;
> +}
> +
> +visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
> +visit_free(v);
> +
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return -EINVAL;
> +}
> +
> +return 0;
> +}
> +
>  static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>   Error **errp)
>  {
>  BDRVRBDState *s = bs->opaque;
>  BlockdevOptionsRbd *opts = NULL;
> -Visitor *v;
>  const QDictEntry *e;
>  Error *local_err = NULL;
>  char *keypairs, *secretid;
> @@ -676,19 +698,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  qdict_del(options, "password-secret");
>  }
>  
> -/* Convert the remaining options into a QAPI object */
> -v = qobject_input_visitor_new_flat_confused(options, errp);
> -if (!v) {
> -r = -EINVAL;
> -goto out;
> -}
> -
> -visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
> -visit_free(v);
> -
> +r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>  if (local_err) {
>  error_propagate(errp, local_err);
> -r = -EINVAL;
>  goto out;
>  }
>  
> 

Reviewed-by: John Snow

Re: [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames

On Tue, Sep 11, 2018 at 01:15:49AM -0400, Jeff Cody wrote:
> When we converted rbd to get rid of the older key/value-centric
> encoding format, we broke compatibility with image files with backing
> file strings encoded in the old format.
> 
> This leaves a bit of an ugly conundrum, and a hacky solution.
> 
> If the initial attempt to parse the "proper" options fails, it assumes
> that we may have an older key/value encoded filename.  Fall back to
> attempting to parse the filename, and extract the required options from
> it.  If that fails, pass along the original error message.
> 
> This approach has a potential drawback: if for some reason there are
> some options supplied the new way, and some the old way, we may not
> catch all the old options if they are not required options (since it
> won't cause the initial failure).
> 
> Signed-off-by: Jeff Cody 
> ---
>  block/rbd.c | 30 +++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index a8e79d01d2..bce86b8bde 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  BlockdevOptionsRbd *opts = NULL;
>  const QDictEntry *e;
>  Error *local_err = NULL;
> -char *keypairs, *secretid;
> +char *keypairs, *secretid, *filename;
>  int r;
>  
>  keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> @@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>  r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>  if (local_err) {
> -error_propagate(errp, local_err);
> -goto out;
> +/* If the initial attempt to convert and process the options failed,
> + * we may be attempting to open an image file that has the rbd 
> options
> + * specified in the older format consisting of all key/value pairs
> + * encoded in the filename.  Go ahead and attempt to parse the
> + * filename, and see if we can pull out the required options */
> +Error *parse_err = NULL;
> +
> +filename = g_strdup(qdict_get_try_str(options, "filename"));

I'm leaking filename, I'll clean that up (along with any other comments) in
v2.

> +qdict_del(options, "filename");
> +
> +qemu_rbd_parse_filename(filename, options, NULL);
> +
> +g_free(keypairs);
> +keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> +if (keypairs) {
> +qdict_del(options, "=keyvalue-pairs");
> +}
> +
> +r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
> +if (parse_err) {
> +/* if the second attempt failed, pass along the original error
> + * message for the current format */
> +error_propagate(errp, local_err);
> +error_free(parse_err);
> +goto out;
> +}
>  }
>  
>  /* Remove the processed options from the QDict (the visitor processes
> -- 
> 2.17.1
>

Re: [Qemu-devel] [PATCH 1/2] block/rbd: pull out qemu_rbd_convert_options


On 9/11/18 12:15 AM, Jeff Cody wrote:

Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
into a helper function.

Signed-off-by: Jeff Cody 
---
  block/rbd.c | 36 
  1 file changed, 24 insertions(+), 12 deletions(-)




-
+r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
  if (local_err) {
  error_propagate(errp, local_err);
-r = -EINVAL;
  goto out;
  }


You could simplify this to:

r = qemu_rbd_convert_options(bs, options, &opts, errp);
if (r < 0) {
goto out;
}

Either way, this refactoring looks correct.

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [Question] Question about ACPI table in qemu

2018-09-11 Thread Paolo Bonzini

On 11/09/2018 18:54, Li Qiang wrote:
> Hi all,
> 
> I noticed that both qemu and seabios create the ACPI table.
> Once I think he bios' ACPI table will overwrite the qemu's if seabios
> compiled with CONFIG_ACPI.

Yes, SeaBIOS's ACPI tables are not used anymore, to remove the need to
update QEMU and SeaBIOS in sync.

> But after I read this -->
> https://lists.gnu.org/archive/html/qemu-devel/2013-02/msg04555.html
> 
> There say:
> 
> "just have QEMU pass the tables to SeaBIOS for it to copy
 into memory like it does on CSM, coreboot, and Xen"
> 
> So where does this 'copy' happens? or is there something others I missed?

The copy happens in SeaBIOS's romfile_loader_execute function (called by
qemu_platform_setup).

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 5/6] target/i386/translate: use thread-local storage in !user-mode

On Tue, Sep 11, 2018 at 13:24:03 +0200, Paolo Bonzini wrote:
> On 10/09/2018 14:30, Emilio G. Cota wrote:
> >> I'm confused - as we can have multi-threaded user space don't the same
> >> requirements apply?
> > In user-mode, code generation is serialized by mmap_lock.
> > Making these per-thread would just waste TLS space.
> 
> It's stupid question time!  How can the TLS work?  tcg_x86_init is only
> called once, the first time cpu_exec_realizefn is called.
>
> Either they can be kept in non-TLS, or you should move them to DisasContext.

Yes, the latter is the Right Thing (tm), as both you and Richard
pointed out. I should have done that in the first place.

Will send a v3 with just this change + the configure patch.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH V2] block: increased maximum size of vvfat devices

2018-09-11 Thread аркадий иванов

Ping?

Thu, 23 Aug. 2018 at 16:37, Arkasha :

> This fixes the problem of the impossibility to create
> FAT16 disks larger than 504 mb:
> The change CHS made it possible to obtain a larger disk.
> Also, auto-detection of disk parameters was added depending
> on the volume of the connected files:
> The size of all folders and files on the created disk is calculated
> and the size of the FAT table is added.
> This size allows to choose the future size of the FAT drive
> from the standard limitations.
>
> Signed-off-by: Arkasha 
> ---
>  block/vvfat.c | 96
> ---
>  1 file changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index fc41841..4409233 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -77,6 +77,47 @@ static void checkpoint(void);
>  #define DIR_KANJI_FAKE 0x05
>  #define DIR_FREE 0x00
>
> +static bool check_size(uint32_t offset_to_bootsector, int cyls, int heads,
> +  int secs, uint8_t sectors_per_cluster, int fat_type,
> +  long int sum, long int size_disk)
> +{
> +uint32_t sector_count = cyls * heads * secs - offset_to_bootsector;
> +int i = 1 + sectors_per_cluster * 0x200 * 8 / fat_type;
> +uint16_t sectors_per_fat = (sector_count + i) / i;
> +/*size + FAT1 and FAT2 table size*/
> +if ((sum + sectors_per_fat * 512 * 2) < size_disk) {
> +return true;
> +}
> +return false;
> +}
> +
> +static long int find_size(const char *dirname, unsigned int cluster)
> +{
> +long int sum = 0;
> +DIR *dir = opendir(dirname);
> +struct dirent *entry;
> +while ((entry = readdir(dir))) {
> +unsigned int length = strlen(dirname) + 2 + strlen(entry->d_name);
> +char *buffer;
> +struct stat st;
> +buffer = g_malloc(length);
> +snprintf(buffer, length, "%s/%s", dirname, entry->d_name);
> +if (stat(buffer, &st) < 0) {
> +g_free(buffer);
> +continue;
> +}
> +if (strcmp(entry->d_name, ".") && strcmp(entry->d_name, "..")
> +&& S_ISDIR(st.st_mode)) {
> +sum += find_size(buffer, cluster);
> +}
> +g_free(buffer);
> +sum += (st.st_size + cluster - 1) / (cluster) * (cluster);
> +}
> +closedir(dir);
> +return sum;
> +}
> +
> +
>  /* dynamic array functions */
>  typedef struct array_t {
>  char* pointer;
> @@ -948,8 +988,6 @@ static int init_directories(BDRVVVFATState* s,
>  /* Now build FAT, and write back information into directory */
>  init_fat(s);
>
> -/* TODO: if there are more entries, bootsector has to be adjusted! */
> -s->root_entries = 0x02 * 0x10 * s->sectors_per_cluster;
>  s->cluster_count=sector2cluster(s, s->sector_count);
>
>  mapping = array_get_next(&(s->mapping));
> @@ -1154,12 +1192,12 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
>  {
>  BDRVVVFATState *s = bs->opaque;
>  int cyls, heads, secs;
> +long int size_disk;
>  bool floppy;
>  const char *dirname, *label;
>  QemuOpts *opts;
>  Error *local_err = NULL;
>  int ret;
> -
>  #ifdef DEBUG
>  vvv = s;
>  #endif
> @@ -1181,6 +1219,29 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
>
>  s->fat_type = qemu_opt_get_number(opts, "fat-type", 0);
>  floppy = qemu_opt_get_bool(opts, "floppy", false);
> +unsigned int cluster;
> +long int sum = 0;
> +if (floppy) {
> +if (!s->fat_type) {
> +s->sectors_per_cluster = 2;
> +} else {
> +s->sectors_per_cluster = 1;
> +}
> +} else if (s->fat_type == 12) {
> +s->offset_to_bootsector = 0x3f;
> +s->sectors_per_cluster = 0x10;
> +} else {
> +s->offset_to_bootsector = 0x3f;
> +/* LATER TODO: if FAT32, adjust */
> +s->sectors_per_cluster = 0x80;
> +}
> +
> +cluster = s->sectors_per_cluster * 0x200;
> +sum += find_size(dirname, cluster);
> +/* TODO: if there are more entries, bootsector has to be adjusted! */
> +s->root_entries = 0x02 * 0x10 * s->sectors_per_cluster;
> +/*File size + boot sector size + root directory size*/
> +sum += 512 + s->root_entries * 32;
>
>  memset(s->volume_label, ' ', sizeof(s->volume_label));
>  label = qemu_opt_get(opts, "label");
> @@ -1201,24 +1262,41 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
>  if (!s->fat_type) {
>  s->fat_type = 12;
>  secs = 36;
> -s->sectors_per_cluster = 2;
>  } else {
>  secs = s->fat_type == 12 ? 18 : 36;
> -s->sectors_per_cluster = 1;
>  }
>  cyls = 80;
>  heads = 2;
>  } else {
> -/* 32MB or 504MB disk*/
>  if (!s->fat_type) {
>  s->fat_type = 16;
>  }
> -s->o

[Qemu-devel] [PATCH 4/6] i386: acpi: add IVHD device entry for IOAPIC

When interrupt remapping is enabled, add a special IVHD device
(type IOAPIC) -- which is typically PCI device 14:0.0. Linux IOMMU driver
checks for this special device.

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 
Signed-off-by: Brijesh Singh 
---
 hw/i386/acpi-build.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index e1ee8ae..5c2c638 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2519,6 +2519,7 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
 static void
 build_amd_iommu(GArray *table_data, BIOSLinker *linker)
 {
+int ivhd_table_len = 28;
 int iommu_start = table_data->len;
 AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
 
@@ -2540,8 +2541,16 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
  (1UL << 6) | /* PrefSup  */
  (1UL << 7),  /* PPRSup   */
  1);
+
+/*
+ * When interrupt remapping is enabled, we add a special IVHD device
+ * for type IO-APIC.
+ */
+if (s->intr_enabled) {
+ivhd_table_len += 8;
+}
 /* IVHD length */
-build_append_int_noprefix(table_data, 28, 2);
+build_append_int_noprefix(table_data, ivhd_table_len, 2);
 /* DeviceID */
 build_append_int_noprefix(table_data, s->devid, 2);
 /* Capability offset */
@@ -2565,6 +2574,15 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
  */
 build_append_int_noprefix(table_data, 0x001, 4);
 
+/*
+ * When interrupt remapping is enabled, Linux IOMMU driver also checks
+ * for special IVHD device (type IO-APIC), which is typically presented
+ * as PCI device 14:00.0.
+ */
+if (s->intr_enabled) {
+build_append_int_noprefix(table_data, 0x0100a048, 8);
+}
+
 build_header(linker, table_data, (void *)(table_data->data + iommu_start),
  "IVRS", table_data->len - iommu_start, 1, NULL, NULL);
 }
-- 
2.7.4

[Qemu-devel] [PATCH 2/6] x86_iommu/amd: Prepare for interrupt remap support

Register the interrupt remapping callback and read/write ops for the
amd-iommu-ir memory region.

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 
Signed-off-by: Brijesh Singh 
---
 hw/i386/amd_iommu.c  | 107 +++
 hw/i386/amd_iommu.h  |  17 +++-
 hw/i386/trace-events |   5 +++
 3 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 1fd669f..572ba0a 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -26,6 +26,7 @@
 #include "amd_iommu.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "hw/i386/apic_internal.h"
 #include "trace.h"
 
 /* used AMD-Vi MMIO registers */
@@ -1026,6 +1027,101 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion 
*iommu, hwaddr addr,
 return ret;
 }
 
+/* Interrupt remapping for MSI/MSI-X entry */
+static int amdvi_int_remap_msi(AMDVIState *iommu,
+   MSIMessage *origin,
+   MSIMessage *translated,
+   uint16_t sid)
+{
+int ret;
+
+assert(origin && translated);
+
+trace_amdvi_ir_remap_msi_req(origin->address, origin->data, sid);
+
+if (!iommu || !iommu->intr_enabled) {
+memcpy(translated, origin, sizeof(*origin));
+goto out;
+}
+
+if (origin->address & AMDVI_MSI_ADDR_HI_MASK) {
+trace_amdvi_err("MSI address high 32 bits non-zero when "
+"Interrupt Remapping enabled.");
+return -AMDVI_IR_ERR;
+}
+
+if ((origin->address & AMDVI_MSI_ADDR_LO_MASK) != APIC_DEFAULT_ADDRESS) {
+trace_amdvi_err("MSI is not from IOAPIC.");
+return -AMDVI_IR_ERR;
+}
+
+out:
+trace_amdvi_ir_remap_msi(origin->address, origin->data,
+ translated->address, translated->data);
+return 0;
+}
+
+static int amdvi_int_remap(X86IOMMUState *iommu,
+   MSIMessage *origin,
+   MSIMessage *translated,
+   uint16_t sid)
+{
+return amdvi_int_remap_msi(AMD_IOMMU_DEVICE(iommu), origin,
+   translated, sid);
+}
+
+static MemTxResult amdvi_mem_ir_write(void *opaque, hwaddr addr,
+  uint64_t value, unsigned size,
+  MemTxAttrs attrs)
+{
+int ret;
+MSIMessage from = { 0, 0 }, to = { 0, 0 };
+uint16_t sid = AMDVI_SB_IOAPIC_ID;
+
+from.address = (uint64_t) addr + AMDVI_INT_ADDR_FIRST;
+from.data = (uint32_t) value;
+
+trace_amdvi_mem_ir_write_req(addr, value, size);
+
+if (!attrs.unspecified) {
+/* We have explicit Source ID */
+sid = attrs.requester_id;
+}
+
+ret = amdvi_int_remap_msi(opaque, &from, &to, sid);
+if (ret < 0) {
+/* TODO: report error */
+/* Drop the interrupt */
+return MEMTX_ERROR;
+}
+
+apic_get_class()->send_msi(&to);
+
+trace_amdvi_mem_ir_write(to.address, to.data);
+return MEMTX_OK;
+}
+
+static MemTxResult amdvi_mem_ir_read(void *opaque, hwaddr addr,
+ uint64_t *data, unsigned size,
+ MemTxAttrs attrs)
+{
+return MEMTX_OK;
+}
+
+static const MemoryRegionOps amdvi_ir_ops = {
+.read_with_attrs = amdvi_mem_ir_read,
+.write_with_attrs = amdvi_mem_ir_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+}
+};
+
 static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
 {
 AMDVIState *s = opaque;
@@ -1055,6 +1151,12 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, 
void *opaque, int devfn)
 address_space_init(&iommu_as[devfn]->as,
MEMORY_REGION(&iommu_as[devfn]->iommu),
"amd-iommu");
+memory_region_init_io(&iommu_as[devfn]->iommu_ir, OBJECT(s),
+  &amdvi_ir_ops, s, "amd-iommu-ir",
+  AMDVI_INT_ADDR_SIZE);
+memory_region_add_subregion(MEMORY_REGION(&iommu_as[devfn]->iommu),
+  AMDVI_INT_ADDR_FIRST,
+  &iommu_as[devfn]->iommu_ir);
 }
 return &iommu_as[devfn]->as;
 }
@@ -1172,6 +1274,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 return;
 }
 
+/* Pseudo address space under root PCI bus. */
+pcms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_SB_IOAPIC_ID);
+s->intr_enabled = x86_iommu->intr_supported;
+
 /* set up MMIO */
 memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
   AMDVI_MMIO_SIZE);
@@ -1205,6 +1311

[Qemu-devel] [PATCH 1/6] x86_iommu: move the kernel-irqchip check in common code

Interrupt remapping needs kernel-irqchip={off|split} on both Intel and AMD
platforms. Move the check in common place.

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 
Signed-off-by: Brijesh Singh 
---
 hw/i386/intel_iommu.c | 7 ---
 hw/i386/x86-iommu.c   | 9 +
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3dfada1..84dbc20 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3248,13 +3248,6 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
 
-/* Currently Intel IOMMU IR only support "kernel-irqchip={off|split}" */
-if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() &&
-!kvm_irqchip_is_split()) {
-error_setg(errp, "Intel Interrupt Remapping cannot work with "
- "kernel-irqchip=on, please use 'split|off'.");
-return false;
-}
 if (s->intr_eim == ON_OFF_AUTO_ON && !x86_iommu->intr_supported) {
 error_setg(errp, "eim=on cannot be selected without intremap=on");
 return false;
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 8a01a2d..7440cb8 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
+#include "sysemu/kvm.h"
 
 void x86_iommu_iec_register_notifier(X86IOMMUState *iommu,
  iec_notify_fn fn, void *data)
@@ -94,6 +95,14 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 return;
 }
 
+/* Both Intel and AMD IOMMU IR only support "kernel-irqchip={off|split}" */
+if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() &&
+!kvm_irqchip_is_split()) {
+error_setg(errp, "Interrupt Remapping cannot work with "
+ "kernel-irqchip=on, please use 'split|off'.");
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
2.7.4

[Qemu-devel] [Question] Question about ACPI table in qemu

2018-09-11 Thread Li Qiang

Hi all,

I noticed that both qemu and seabios create the ACPI table.
Once I think he bios' ACPI table will overwrite the qemu's if seabios
compiled with CONFIG_ACPI.

But after I read this -->
https://lists.gnu.org/archive/html/qemu-devel/2013-02/msg04555.html

There say:

"just have QEMU pass the tables to SeaBIOS for it to copy
>>> into memory like it does on CSM, coreboot, and Xen"

So where does this 'copy' happens? or is there something others I missed?

Thanks,
Li Qiang

[Qemu-devel] [PATCH 3/6] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled

Emulate the interrupt remapping support when guest virtual APIC is
not enabled.

See IOMMU spec: https://support.amd.com/TechDocs/48882_IOMMU.pdf
(section 2.2.5.1) for details information.

When VAPIC is not enabled, it uses interrupt remapping as defined in
Table 20 and Figure 15 from IOMMU spec.

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 
Signed-off-by: Brijesh Singh 
---
 hw/i386/amd_iommu.c  | 187 +++
 hw/i386/amd_iommu.h  |  60 -
 hw/i386/trace-events |   7 ++
 3 files changed, 253 insertions(+), 1 deletion(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 572ba0a..5ac19df 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -28,6 +28,8 @@
 #include "qemu/error-report.h"
 #include "hw/i386/apic_internal.h"
 #include "trace.h"
+#include "cpu.h"
+#include "hw/i386/apic-msidef.h"
 
 /* used AMD-Vi MMIO registers */
 const char *amdvi_mmio_low[] = {
@@ -1027,6 +1029,119 @@ static IOMMUTLBEntry amdvi_translate(IOMMUMemoryRegion 
*iommu, hwaddr addr,
 return ret;
 }
 
+static int amdvi_get_irte(AMDVIState *s, MSIMessage *origin, uint64_t *dte,
+  union irte *irte, uint16_t devid)
+{
+uint64_t irte_root, offset;
+
+irte_root = dte[2] & AMDVI_IR_PHYS_ADDR_MASK;
+offset = (origin->data & AMDVI_IRTE_OFFSET) << 2;
+
+trace_amdvi_ir_irte(irte_root, offset);
+
+if (dma_memory_read(&address_space_memory, irte_root + offset,
+irte, sizeof(*irte))) {
+trace_amdvi_ir_err("failed to get irte");
+return -AMDVI_IR_GET_IRTE;
+}
+
+trace_amdvi_ir_irte_val(irte->val);
+
+return 0;
+}
+
+static void amdvi_generate_msi_message(struct AMDVIIrq *irq, MSIMessage *out)
+{
+out->address = APIC_DEFAULT_ADDRESS | \
+(irq->dest_mode << MSI_ADDR_DEST_MODE_SHIFT) | \
+(irq->redir_hint << MSI_ADDR_REDIRECTION_SHIFT) | \
+(irq->dest << MSI_ADDR_DEST_ID_SHIFT);
+
+out->data = (irq->vector << MSI_DATA_VECTOR_SHIFT) | \
+(irq->delivery_mode << MSI_DATA_DELIVERY_MODE_SHIFT);
+
+trace_amdvi_ir_generate_msi_message(irq->vector, irq->delivery_mode,
+irq->dest_mode, irq->dest, irq->redir_hint);
+}
+
+static int amdvi_int_remap_legacy(AMDVIState *iommu,
+  MSIMessage *origin,
+  MSIMessage *translated,
+  uint64_t *dte,
+  struct AMDVIIrq *irq,
+  uint16_t sid)
+{
+int ret;
+union irte irte;
+
+/* get interrupt remapping table */
+ret = amdvi_get_irte(iommu, origin, dte, &irte, sid);
+if (ret < 0) {
+return ret;
+}
+
+if (!irte.fields.valid) {
+trace_amdvi_ir_target_abort("RemapEn is disabled");
+return -AMDVI_IR_TARGET_ABORT;
+}
+
+if (irte.fields.guest_mode) {
+trace_amdvi_ir_target_abort("guest mode is not zero");
+return -AMDVI_IR_TARGET_ABORT;
+}
+
+if (irte.fields.int_type > AMDVI_IOAPIC_INT_TYPE_ARBITRATED) {
+trace_amdvi_ir_target_abort("reserved int_type");
+return -AMDVI_IR_TARGET_ABORT;
+}
+
+irq->delivery_mode = irte.fields.int_type;
+irq->vector = irte.fields.vector;
+irq->dest_mode = irte.fields.dm;
+irq->redir_hint = irte.fields.rq_eoi;
+irq->dest = irte.fields.destination;
+
+return 0;
+}
+
+static int __amdvi_int_remap_msi(AMDVIState *iommu,
+ MSIMessage *origin,
+ MSIMessage *translated,
+ uint64_t *dte,
+ uint16_t sid)
+{
+int ret;
+uint8_t int_ctl;
+struct AMDVIIrq irq = { 0 };
+
+int_ctl = (dte[2] >> AMDVI_IR_INTCTL_SHIFT) & 3;
+trace_amdvi_ir_intctl(int_ctl);
+
+switch (int_ctl) {
+case AMDVI_IR_INTCTL_PASS:
+memcpy(translated, origin, sizeof(*origin));
+return 0;
+case AMDVI_IR_INTCTL_REMAP:
+break;
+case AMDVI_IR_INTCTL_ABORT:
+trace_amdvi_ir_target_abort("int_ctl abort");
+return -AMDVI_IR_TARGET_ABORT;
+default:
+trace_amdvi_ir_target_abort("int_ctl reserved");
+return -AMDVI_IR_TARGET_ABORT;
+}
+
+ret = amdvi_int_remap_legacy(iommu, origin, translated, dte, &irq, sid);
+if (ret < 0) {
+return ret;
+}
+
+/* Translate AMDVIIrq to MSI message */
+amdvi_generate_msi_message(&irq, translated);
+
+return 0;
+}
+
 /* Interrupt remapping for MSI/MSI-X entry */
 static int amdvi_int_remap_msi(AMDVIState *iommu,
MSIMessage *origin,
@@ -1034,6 +1149,9 @@ static int amdvi_int_remap_msi(AMDVIState *iommu,
uint16_t sid)
 {
 int ret;
+uint64_t pass = 0;
+uint64_t dte[4] =

[Qemu-devel] [PATCH 0/6] x86_iommu/amd: add interrupt remap support

This series adds the interrupt remapping support for amd-iommu device.

IOMMU spec is available at: https://support.amd.com/TechDocs/48882_IOMMU.pdf

To enable the interrupt remap use below qemu cli
# $QEMU \
  -device amd-iommu,intremap=on

I have tested FC-28 and Ubuntu 18.04 guest. 

Linux guest bootup log shows the interrupt remap supports:

[root@localhost ~]# dmesg | grep -i AMD-Vi
[0.001761] AMD-Vi: Using IVHD type 0x10
[0.003051] AMD-Vi: device: 00:03.0 cap: 0040 seg: 0 flags: d1 info 
[0.004007] AMD-Vi:mmio-addr: fed8
[0.004874] AMD-Vi:   DEV_ALLflags: 00
[0.006236] AMD-Vi:   DEV_SPECIAL(IOAPIC[0]) devid: 00:14.0
[0.667943] AMD-Vi: Found IOMMU at :00:03.0 cap 0x40
[0.668727] AMD-Vi: Extended features (0x29d3):
[0.669874] AMD-Vi: Interrupt remapping enabled
[0.671074] AMD-Vi: Lazy IO/TLB flushing enabled

cat /proc/interrupts confirms that its using IR

[root@localhost ~]# cat /proc/interrupts 
CPU0   
 0: 40  IR-IO-APIC2-edge  timer
 1:  9  IR-IO-APIC1-edge  i8042
 4:   1770  IR-IO-APIC4-edge  ttyS0
 7:  0  IR-IO-APIC7-edge  parport0
 8:  1  IR-IO-APIC8-edge  rtc0
 9:  0  IR-IO-APIC9-fasteoi   acpi
12: 15  IR-IO-APIC   12-edge  i8042
16:  0  IR-IO-APIC   16-fasteoi   i801_smbus
24:  0   PCI-MSI 49152-edge  AMD-Vi
25:  13070  IR-PCI-MSI 512000-edge  ahci[:00:1f.2]
26: 86  IR-PCI-MSI 32768-edge  enp0s2-rx-0
27:139  IR-PCI-MSI 32769-edge  enp0s2-tx-0
28:  1  IR-PCI-MSI 32770-edge  enp0s2
NMI:  0   Non-maskable interrupts
LOC:  26686   Local timer interrupts
SPU:  0   Spurious interrupts
...
...

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 

Brijesh Singh (6):
  x86_iommu: move the kernel-irqchip check in common code
  x86_iommu/amd: Prepare for interrupt remap support
  x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled
  i386: acpi: add IVHD device entry for IOAPIC
  x86_iommu/amd: Add interrupt remap support when VAPIC is enabled
  x86_iommu/amd: Enable Guest virtual APIC support

 hw/i386/acpi-build.c  |  23 +++-
 hw/i386/amd_iommu.c   | 360 ++
 hw/i386/amd_iommu.h   | 115 +++-
 hw/i386/intel_iommu.c |   7 -
 hw/i386/trace-events  |  14 ++
 hw/i386/x86-iommu.c   |   9 ++
 6 files changed, 516 insertions(+), 12 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 6/6] x86_iommu/amd: Enable Guest virtual APIC support

Now that amd-iommu support interrupt remapping, enable the GASup in IVRS
table and GASup in extended feature register to indicate that IOMMU
support guest virtual APIC mode.

Note that the GAMSup is set to zero to indicate that  Guest Virtual
APIC does not support advanced interrupt features (i.e virtualized
interrupts using the guest virtual APIC).

See Table 21 from IOMMU spec for interrupt virtualization controls

IOMMU spec: https://support.amd.com/TechDocs/48882_IOMMU.pdf

Cc: "Michael S. Tsirkin" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcel Apfelbaum 
Cc: Tom Lendacky 
Cc: Suravee Suthikulpanit 
Signed-off-by: Brijesh Singh 
---
 hw/i386/acpi-build.c | 3 ++-
 hw/i386/amd_iommu.h  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5c2c638..1cbc8ba 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2565,7 +2565,8 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker)
 build_append_int_noprefix(table_data,
  (48UL << 30) | /* HATS   */
  (48UL << 28) | /* GATS   */
- (1UL << 2),/* GTSup  */
+ (1UL << 2)   | /* GTSup  */
+ (1UL << 6),/* GASup  */
  4);
 /*
  *   Type 1 device entry reporting all devices
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 1dab974..5defaac 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -177,7 +177,7 @@
 /* extended feature support */
 #define AMDVI_EXT_FEATURES (AMDVI_FEATURE_PREFETCH | AMDVI_FEATURE_PPR | \
 AMDVI_FEATURE_IA | AMDVI_FEATURE_GT | AMDVI_FEATURE_HE | \
-AMDVI_GATS_MODE | AMDVI_HATS_MODE)
+AMDVI_GATS_MODE | AMDVI_HATS_MODE | AMDVI_FEATURE_GA)
 
 /* capabilities header */
 #define AMDVI_CAPAB_FEATURES (AMDVI_CAPAB_FLAT_EXT | \
-- 
2.7.4

[Qemu-devel] [Bug 1791796] Re: unimplemented thread syscalls in nios2 user-mode emulation

2018-09-11 Thread Sandra Loosemore

If you need a Nios II GNU/Linux toolchain, I think the most recent
CodeBench Lite release will work:

https://sourcery.mentor.com/GNUToolchain/subscription42545

We're planning on adding user-mode QEMU to the upcoming 2018.11
release  that's actually what I've been testing it for.  Results on
the GCC testsuite actually don't look too bad, but I have a patch I
haven't submitted yet that's required to make the GDB stub work, and
there are a lot of GDB test failures I haven't triaged yet.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1791796

Title:
  unimplemented thread syscalls in nios2 user-mode emulation

Status in QEMU:
  New

Bug description:
  This bug is reported against the 3.0 release.

  I noticed that the GCC test gcc.dg/torture/tls/tls-test.c is failing
  when run in user-mode qemu for nios2 target.  The problem appears to
  be that the thread-related syscalls are unimplemented in qemu.  Here
  is output from running with -strace:

  22484 brk(NULL) = 0x5000
  22484 uname(0x7fffef5a) = 0
  22484 faccessat(AT_FDCWD,"/etc/ld.so.preload",R_OK,0x5) = -1 errno=2 (No such 
file or directory)
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./tls/libm.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = -1 errno=2 (No such file or directory)
  22484 
fstatat64(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./tls",0x7fffe870,0)
 = -1 errno=2 (No such file or directory)
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libm.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe954,512) = 512
  22484 fstat64(3,0x7fffe870) = 0
  22484 mmap2(NULL,803596,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f716000
  22484 
mmap2(0x7f7d8000,12288,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0xc1)
 = 0x7f7d8000
  22484 close(3) = 0
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libpthread.so.0",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe948,512) = 512
  22484 mmap2(NULL,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x7f714000
  22484 fstat64(3,0x7fffe864) = 0
  22484 mmap2(NULL,120700,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f6f6000
  22484 mprotect(0x7f70e000,4096,PROT_NONE) = 0
  22484 
mmap2(0x7f70f000,12288,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x18)
 = 0x7f70f000
  22484 
mmap2(0x7f712000,6012,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
 = 0x7f712000
  22484 close(3) = 0
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libc.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe93c,512) = 512
  22484 fstat64(3,0x7fffe858) = 0
  22484 mmap2(NULL,1491048,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f589000
  22484 
mmap2(0x7f6de000,86016,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x154)
 = 0x7f6de000
  22484 
mmap2(0x7f6f3000,8296,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
 = 0x7f6f3000
  22484 close(3) = 0
  22484 mprotect(0x7f6de000,65536,PROT_READ) = 0
  22484 mprotect(0x7f70f000,8192,PROT_READ) = 0
  22484 mprotect(0x7f7d8000,4096,PROT_READ) = 0
  22484 mprotect(0x3000,4096,PROT_READ) = 0
  22484 mprotect(0x7f7fc000,4096,PROT_READ) = 0
  22484 set_tid_address(2138131700,2147480980,2147480988,2147480988,87148,47) = 
22484
  22484 set_robust_list(2138131708,12,2147480988,0,87148,47) = -1 errno=38 
(Function not implemented)
  22484 rt_sigaction(32,0x736c,NULL) = 0
  22484 rt_sigaction(33,0x736c,NULL) = -1 errno=22 (Invalid argument)
  22484 rt_sigprocmask(SIG_UNBLOCK,0x74a8,NULL) = 0
  22484 getrlimit(3,2147480732,3,0,62512,47) = 0
  22484 mmap2(NULL,8392704,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|0x2,-1,0) = 
0x7ed88000
  22484 mprotect(0x7ed89000,8388608,PROT_READ|PROT_WRITE) = 0
  22484 brk(NULL) = 0x5000
  22484 brk(0x00026000) = 0x00026000
  22484 
clone(CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,child_stack=0x7f588018,parent_tidptr=0x7f5884fc,tls=0x7f58f928,child_tidptr=0x7f5884fc)
 = 22503
  22484 io_setup(4001536,2136506392,2136507644,2136507644,2136537384,4100) = -1 
errno=38 (Function not implemented)
  22484 futex(0x7f5884fc,FUTEX_WAIT,22503,NULL,NULL,0)22484 
set_robust_list(2136507652,12,0,4100,2136508076,4100) = -1 errno=38 (Function 
not im

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler

2018-09-11 Thread Paolo Bonzini

On 11/09/2018 16:12, Fam Zheng wrote:
> On Tue, 09/11 13:32, Paolo Bonzini wrote:
>> On 10/09/2018 16:56, Fam Zheng wrote:
>>> We have this unwanted call stack:
>>>
>>>   > ...
>>>   > #13 0x5586602b7793 in virtio_scsi_handle_cmd_vq
>>>   > #14 0x5586602b8d66 in virtio_scsi_data_plane_handle_cmd
>>>   > #15 0x5586602ddab7 in virtio_queue_notify_aio_vq
>>>   > #16 0x5586602dfc9f in virtio_queue_host_notifier_aio_poll
>>>   > #17 0x5586607885da in run_poll_handlers_once
>>>   > #18 0x55866078880e in try_poll_mode
>>>   > #19 0x5586607888eb in aio_poll
>>>   > #20 0x558660784561 in aio_wait_bh_oneshot
>>>   > #21 0x5586602b9582 in virtio_scsi_dataplane_stop
>>>   > #22 0x5586605a7110 in virtio_bus_stop_ioeventfd
>>>   > #23 0x5586605a9426 in virtio_pci_stop_ioeventfd
>>>   > #24 0x5586605ab808 in virtio_pci_common_write
>>>   > #25 0x558660242396 in memory_region_write_accessor
>>>   > #26 0x5586602425ab in access_with_adjusted_size
>>>   > #27 0x558660245281 in memory_region_dispatch_write
>>>   > #28 0x5586601e008e in flatview_write_continue
>>>   > #29 0x5586601e01d8 in flatview_write
>>>   > #30 0x5586601e04de in address_space_write
>>>   > #31 0x5586601e052f in address_space_rw
>>>   > #32 0x5586602607f2 in kvm_cpu_exec
>>>   > #33 0x558660227148 in qemu_kvm_cpu_thread_fn
>>>   > #34 0x55866078bde7 in qemu_thread_start
>>>   > #35 0x7f5784906594 in start_thread
>>>   > #36 0x7f5784639e6f in clone
>>>
>>> Avoid it with the aio_disable_external/aio_enable_external pair, so that
>>> no vq poll handlers can be called in aio_wait_bh_oneshot.
>>
>> I don't understand.  We are in the vCPU thread, so not in the
>> AioContext's home thread.  Why is aio_wait_bh_oneshot polling rather
>> than going through the aio_wait_bh path?
> 
> What do you mean by 'aio_wait_bh path'? Here is aio_wait_bh_oneshot:

Sorry, I meant the "atomic_inc(&wait_->num_waiters);" path.  But if this
backtrace is obtained without dataplane, that's the answer I was seeking.

> void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> {
> AioWaitBHData data = {
> .cb = cb,
> .opaque = opaque,
> };
> 
> assert(qemu_get_current_aio_context() == qemu_get_aio_context());
> 
> aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
> AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
> }
> 
> ctx is qemu_aio_context here, so there's no interaction with IOThread.

In this case, it should be okay to have the reentrancy, what is the bug
that this patch is fixing?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] configure: Support --enable-capstone=internal

2018-09-11 Thread Eduardo Habkost

On Tue, Sep 11, 2018 at 07:33:08AM -0700, Richard Henderson wrote:
> On 09/05/2018 08:19 AM, Eduardo Habkost wrote:
> > Currently there's no way to make configure not try to use the
> > system-provided capstone library using pkgconfig.
> 
> Certainly there is.
> 
> > 
> > Add support to --enable-capstone=internal option to make QEMU not
> > use the system-provided library automatically.

Oops, the commit message is inaccurate: we can prevent
./configure from trying the system library, but there's no way to
make it avoid using git submodule at the same time.


> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  configure | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index 58862d2ae8..34ed00f6d9 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1431,6 +1431,8 @@ for opt do
> > +  --enable-capstone[=LOCATION]
> > +   Where to look for capstone library.
> > +   Supported options: internal, git, system
> 
> That's what "git" means here.

"git" makes ./configure add capstone to GIT_SUBMODULES, and I'm
pretty sure we don't want that if we're not building from a git
tree.

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

2018-09-11 Thread Marek Vasut

On 09/11/2018 05:08 PM, Alex Bennée wrote:
> 
> Marek Vasut  writes:
> 
>> On 09/11/2018 04:14 PM, Laurent Vivier wrote:
>>> Le 11/09/2018 à 16:06, Alex Bennée a écrit:
 This is to work around the limitations of the buildroot
 qemu_nios2_10m50_defconfig which sets the base kernel version for
 glibc.

 Signed-off-by: Alex Bennée 
 ---
  linux-user/nios2/target_syscall.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/linux-user/nios2/target_syscall.h 
 b/linux-user/nios2/target_syscall.h
 index ca6b7e69f6..905b80d112 100644
 --- a/linux-user/nios2/target_syscall.h
 +++ b/linux-user/nios2/target_syscall.h
 @@ -2,7 +2,7 @@
  #define TARGET_SYSCALL_H

  #define UNAME_MACHINE "nios2"
 -#define UNAME_MINIMUM_RELEASE "3.19.0"
 +#define UNAME_MINIMUM_RELEASE "4.16.0"

  struct target_pt_regs {
  unsigned long  r8;/* r8-r15 Caller-saved GP registers */

>>>
>>> I have no objection. Perhaps you could ask NiosII Maintainers (cc).
>>
>> If that's needed, so be it. The Linux 3.19 was required because some
>> obscure ABI change happened at that point.
> 
> I don't think so - it's an artefact of the way the buildroot toolchain
> is built. But the real question which I address in the cover letter is
> does nios2-linux-user get much use? I tried enabled tests/tcg for it and
> it fails rather badly.

I used it around 2.10 and it worked for me.

-- 
Best regards,
Marek Vasut

Re: [Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]



Marek Vasut  writes:

> On 09/11/2018 04:14 PM, Laurent Vivier wrote:
>> Le 11/09/2018 à 16:06, Alex Bennée a écrit:
>>> This is to work around the limitations of the buildroot
>>> qemu_nios2_10m50_defconfig which sets the base kernel version for
>>> glibc.
>>>
>>> Signed-off-by: Alex Bennée 
>>> ---
>>>  linux-user/nios2/target_syscall.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/linux-user/nios2/target_syscall.h 
>>> b/linux-user/nios2/target_syscall.h
>>> index ca6b7e69f6..905b80d112 100644
>>> --- a/linux-user/nios2/target_syscall.h
>>> +++ b/linux-user/nios2/target_syscall.h
>>> @@ -2,7 +2,7 @@
>>>  #define TARGET_SYSCALL_H
>>>
>>>  #define UNAME_MACHINE "nios2"
>>> -#define UNAME_MINIMUM_RELEASE "3.19.0"
>>> +#define UNAME_MINIMUM_RELEASE "4.16.0"
>>>
>>>  struct target_pt_regs {
>>>  unsigned long  r8;/* r8-r15 Caller-saved GP registers */
>>>
>>
>> I have no objection. Perhaps you could ask NiosII Maintainers (cc).
>
> If that's needed, so be it. The Linux 3.19 was required because some
> obscure ABI change happened at that point.

I don't think so - it's an artefact of the way the buildroot toolchain
is built. But the real question which I address in the cover letter is
does nios2-linux-user get much use? I tried enabled tests/tcg for it and
it fails rather badly.

--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

2018-09-11 Thread Laurent Vivier

Le 11/09/2018 à 16:40, Alex Bennée a écrit :
> 
> Laurent Vivier  writes:
> 
>> Le 11/09/2018 à 16:06, Alex Bennée a écrit:
>>> This is to work around the limitations of the buildroot
>>> qemu_nios2_10m50_defconfig which sets the base kernel version for
>>> glibc.
>>>
>>> Signed-off-by: Alex Bennée 
>>> ---
>>>  linux-user/nios2/target_syscall.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/linux-user/nios2/target_syscall.h 
>>> b/linux-user/nios2/target_syscall.h
>>> index ca6b7e69f6..905b80d112 100644
>>> --- a/linux-user/nios2/target_syscall.h
>>> +++ b/linux-user/nios2/target_syscall.h
>>> @@ -2,7 +2,7 @@
>>>  #define TARGET_SYSCALL_H
>>>
>>>  #define UNAME_MACHINE "nios2"
>>> -#define UNAME_MINIMUM_RELEASE "3.19.0"
>>> +#define UNAME_MINIMUM_RELEASE "4.16.0"
>>>
>>>  struct target_pt_regs {
>>>  unsigned long  r8;/* r8-r15 Caller-saved GP registers */
>>>
>>
>> I have no objection. Perhaps you could ask NiosII Maintainers (cc).
> 
> Doh.. I had cccmd = scripts/get_maintainer.pl --nogit-fallback but of
> course as I didn't actually touch an nios2 files it didn't include them.

I also use "git blame" to know who to bother ;)

Thanks,
Laurent

[Qemu-devel] [Bug 1791796] Re: [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

Laurent Vivier  writes:

> Le 11/09/2018 à 16:06, Alex Bennée a écrit:
>> This is to work around the limitations of the buildroot
>> qemu_nios2_10m50_defconfig which sets the base kernel version for
>> glibc.
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  linux-user/nios2/target_syscall.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/nios2/target_syscall.h 
>> b/linux-user/nios2/target_syscall.h
>> index ca6b7e69f6..905b80d112 100644
>> --- a/linux-user/nios2/target_syscall.h
>> +++ b/linux-user/nios2/target_syscall.h
>> @@ -2,7 +2,7 @@
>>  #define TARGET_SYSCALL_H
>>
>>  #define UNAME_MACHINE "nios2"
>> -#define UNAME_MINIMUM_RELEASE "3.19.0"
>> +#define UNAME_MINIMUM_RELEASE "4.16.0"
>>
>>  struct target_pt_regs {
>>  unsigned long  r8;/* r8-r15 Caller-saved GP registers */
>>
>
> I have no objection. Perhaps you could ask NiosII Maintainers (cc).

Doh.. I had cccmd = scripts/get_maintainer.pl --nogit-fallback but of
course as I didn't actually touch an nios2 files it didn't include them.

Thanks.

>
> Laurent


--
Alex Bennée

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1791796

Title:
  unimplemented thread syscalls in nios2 user-mode emulation

Status in QEMU:
  New

Bug description:
  This bug is reported against the 3.0 release.

  I noticed that the GCC test gcc.dg/torture/tls/tls-test.c is failing
  when run in user-mode qemu for nios2 target.  The problem appears to
  be that the thread-related syscalls are unimplemented in qemu.  Here
  is output from running with -strace:

  22484 brk(NULL) = 0x5000
  22484 uname(0x7fffef5a) = 0
  22484 faccessat(AT_FDCWD,"/etc/ld.so.preload",R_OK,0x5) = -1 errno=2 (No such 
file or directory)
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./tls/libm.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = -1 errno=2 (No such file or directory)
  22484 
fstatat64(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./tls",0x7fffe870,0)
 = -1 errno=2 (No such file or directory)
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libm.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe954,512) = 512
  22484 fstat64(3,0x7fffe870) = 0
  22484 mmap2(NULL,803596,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f716000
  22484 
mmap2(0x7f7d8000,12288,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0xc1)
 = 0x7f7d8000
  22484 close(3) = 0
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libpthread.so.0",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe948,512) = 512
  22484 mmap2(NULL,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x7f714000
  22484 fstat64(3,0x7fffe864) = 0
  22484 mmap2(NULL,120700,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f6f6000
  22484 mprotect(0x7f70e000,4096,PROT_NONE) = 0
  22484 
mmap2(0x7f70f000,12288,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x18)
 = 0x7f70f000
  22484 
mmap2(0x7f712000,6012,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
 = 0x7f712000
  22484 close(3) = 0
  22484 
openat(AT_FDCWD,"/scratch/sandra/nios2-linux-trunk3/obj/test-2018.11-99-nios2-linux-gnu/host-x86_64-linux-gnu/sourceryg++-2018.11/nios2-linux-gnu/libc/./lib/./libc.so.6",O_RDONLY|O_LARGEFILE|O_CLOEXEC)
 = 3
  22484 read(3,0x7fffe93c,512) = 512
  22484 fstat64(3,0x7fffe858) = 0
  22484 mmap2(NULL,1491048,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7f589000
  22484 
mmap2(0x7f6de000,86016,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x154)
 = 0x7f6de000
  22484 
mmap2(0x7f6f3000,8296,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
 = 0x7f6f3000
  22484 close(3) = 0
  22484 mprotect(0x7f6de000,65536,PROT_READ) = 0
  22484 mprotect(0x7f70f000,8192,PROT_READ) = 0
  22484 mprotect(0x7f7d8000,4096,PROT_READ) = 0
  22484 mprotect(0x3000,4096,PROT_READ) = 0
  22484 mprotect(0x7f7fc000,4096,PROT_READ) = 0
  22484 set_tid_address(2138131700,2147480980,2147480988,2147480988,87148,47) = 
22484
  22484 set_robust_list(2138131708,12,2147480988,0,87148,47) = -1 errno=38 
(Function not implemented)
  22484 rt_sigaction(32,0x736c,NULL) = 0
  22484 rt_sigaction(33,0x736c,NULL) = -1 errno=22 (Invalid argument)
  22484 rt_sigprocmask(SIG_UNBLOCK,0x74a8,NULL) = 0
  22484 getrlimit(3,2147480732,3,0,62512,47) = 0
  22484 mmap2(NULL,8392704,PROT_NONE,MAP_PRIVATE|MAP_ANONYMOUS|0x2,-1,0) = 
0x7ed88000
  22484 mprotect(0x7ed89000,83886

Re: [Qemu-devel] [PATCH] configure: Support --enable-capstone=internal

On 09/05/2018 08:19 AM, Eduardo Habkost wrote:
> Currently there's no way to make configure not try to use the
> system-provided capstone library using pkgconfig.

Certainly there is.

> 
> Add support to --enable-capstone=internal option to make QEMU not
> use the system-provided library automatically.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  configure | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/configure b/configure
> index 58862d2ae8..34ed00f6d9 100755
> --- a/configure
> +++ b/configure
> @@ -1431,6 +1431,8 @@ for opt do
> +  --enable-capstone[=LOCATION]
> +   Where to look for capstone library.
> +   Supported options: internal, git, system

That's what "git" means here.


r~

Re: [Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

2018-09-11 Thread Marek Vasut

On 09/11/2018 04:14 PM, Laurent Vivier wrote:
> Le 11/09/2018 à 16:06, Alex Bennée a écrit :
>> This is to work around the limitations of the buildroot
>> qemu_nios2_10m50_defconfig which sets the base kernel version for
>> glibc.
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  linux-user/nios2/target_syscall.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/nios2/target_syscall.h 
>> b/linux-user/nios2/target_syscall.h
>> index ca6b7e69f6..905b80d112 100644
>> --- a/linux-user/nios2/target_syscall.h
>> +++ b/linux-user/nios2/target_syscall.h
>> @@ -2,7 +2,7 @@
>>  #define TARGET_SYSCALL_H
>>  
>>  #define UNAME_MACHINE "nios2"
>> -#define UNAME_MINIMUM_RELEASE "3.19.0"
>> +#define UNAME_MINIMUM_RELEASE "4.16.0"
>>  
>>  struct target_pt_regs {
>>  unsigned long  r8;/* r8-r15 Caller-saved GP registers */
>>
> 
> I have no objection. Perhaps you could ask NiosII Maintainers (cc).

If that's needed, so be it. The Linux 3.19 was required because some
obscure ABI change happened at that point.

-- 
Best regards,
Marek Vasut

Re: [Qemu-devel] [PATCH v10 6/6] tpm: add ACPI memory clear interface

2018-09-11 Thread Laszlo Ersek

+Alex, due to mention of 21e00fa55f3fd

On 09/10/18 15:03, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 10, 2018 at 2:44 PM Dr. David Alan Gilbert
>  wrote:
>> (I didn't know about guest_phys_block* and would have probably just used
>> qemu_ram_foreach_block )
>>
> 
> guest_phys_block*() seems to fit, as it lists only the blocks actually
> used, and already skip the device RAM.
> 
> Laszlo, you wrote the functions
> (https://git.qemu.org/?p=qemu.git;a=commit;h=c5d7f60f0614250bd925071e25220ce5958f75d0),
> do you think it's appropriate to list the memory to clear, or we
> should rather use qemu_ram_foreach_block() ?

Originally, I would have said, "use either, doesn't matter". Namely,
when I introduced the guest_phys_block*() functions, the original
purpose was not related to RAM *contents*, but to RAM *addresses*
(GPAs). This is evident if you look at the direct child commit of
c5d7f60f0614, namely 56c4bfb3f07f, which put GuestPhysBlockList to use.
And, for your use case (= wiping RAM), GPAs don't matter, only contents
matter.

However, with the commits I mentioned previously, namely e4dc3f5909ab9
and 21e00fa55f3fd, we now filter out some RAM blocks from the dumping
based on contents / backing as well. I think? So I believe we should
honor that for the wiping to. I guess I'd (vaguely) suggest using
guest_phys_block*().

(And then, as Dave suggests, maybe extend the filter to consider pmem
too, separately.)

Laszlo

Re: [Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

2018-09-11 Thread Laurent Vivier

Le 11/09/2018 à 16:06, Alex Bennée a écrit :
> This is to work around the limitations of the buildroot
> qemu_nios2_10m50_defconfig which sets the base kernel version for
> glibc.
> 
> Signed-off-by: Alex Bennée 
> ---
>  linux-user/nios2/target_syscall.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/nios2/target_syscall.h 
> b/linux-user/nios2/target_syscall.h
> index ca6b7e69f6..905b80d112 100644
> --- a/linux-user/nios2/target_syscall.h
> +++ b/linux-user/nios2/target_syscall.h
> @@ -2,7 +2,7 @@
>  #define TARGET_SYSCALL_H
>  
>  #define UNAME_MACHINE "nios2"
> -#define UNAME_MINIMUM_RELEASE "3.19.0"
> +#define UNAME_MINIMUM_RELEASE "4.16.0"
>  
>  struct target_pt_regs {
>  unsigned long  r8;/* r8-r15 Caller-saved GP registers */
> 

I have no objection. Perhaps you could ask NiosII Maintainers (cc).

Laurent

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi/virtio-blk: Disable poll handlers when stopping vq handler

2018-09-11 Thread Fam Zheng

On Tue, 09/11 13:32, Paolo Bonzini wrote:
> On 10/09/2018 16:56, Fam Zheng wrote:
> > We have this unwanted call stack:
> > 
> >   > ...
> >   > #13 0x5586602b7793 in virtio_scsi_handle_cmd_vq
> >   > #14 0x5586602b8d66 in virtio_scsi_data_plane_handle_cmd
> >   > #15 0x5586602ddab7 in virtio_queue_notify_aio_vq
> >   > #16 0x5586602dfc9f in virtio_queue_host_notifier_aio_poll
> >   > #17 0x5586607885da in run_poll_handlers_once
> >   > #18 0x55866078880e in try_poll_mode
> >   > #19 0x5586607888eb in aio_poll
> >   > #20 0x558660784561 in aio_wait_bh_oneshot
> >   > #21 0x5586602b9582 in virtio_scsi_dataplane_stop
> >   > #22 0x5586605a7110 in virtio_bus_stop_ioeventfd
> >   > #23 0x5586605a9426 in virtio_pci_stop_ioeventfd
> >   > #24 0x5586605ab808 in virtio_pci_common_write
> >   > #25 0x558660242396 in memory_region_write_accessor
> >   > #26 0x5586602425ab in access_with_adjusted_size
> >   > #27 0x558660245281 in memory_region_dispatch_write
> >   > #28 0x5586601e008e in flatview_write_continue
> >   > #29 0x5586601e01d8 in flatview_write
> >   > #30 0x5586601e04de in address_space_write
> >   > #31 0x5586601e052f in address_space_rw
> >   > #32 0x5586602607f2 in kvm_cpu_exec
> >   > #33 0x558660227148 in qemu_kvm_cpu_thread_fn
> >   > #34 0x55866078bde7 in qemu_thread_start
> >   > #35 0x7f5784906594 in start_thread
> >   > #36 0x7f5784639e6f in clone
> > 
> > Avoid it with the aio_disable_external/aio_enable_external pair, so that
> > no vq poll handlers can be called in aio_wait_bh_oneshot.
> 
> I don't understand.  We are in the vCPU thread, so not in the
> AioContext's home thread.  Why is aio_wait_bh_oneshot polling rather
> than going through the aio_wait_bh path?

What do you mean by 'aio_wait_bh path'? Here is aio_wait_bh_oneshot:

void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
{
AioWaitBHData data = {
.cb = cb,
.opaque = opaque,
};

assert(qemu_get_current_aio_context() == qemu_get_aio_context());

aio_bh_schedule_oneshot(ctx, aio_wait_bh, &data);
AIO_WAIT_WHILE(&data.wait, ctx, !data.done);
}

ctx is qemu_aio_context here, so there's no interaction with IOThread. This is
the full backtrace:

https://paste.ubuntu.com/p/Dm7zGzmmRG/

Fam

[Qemu-devel] [RFC PATCH 1/4] docker: add debian-buildroot-base

We can build some more cross-compilers using buildroot. This base
system contains simply the minimum number of tools required for
buildroot to work. We also download and unpack the buildroot source
tree as that will be common for all system deriving from it.

Signed-off-by: Alex Bennée 
---
 tests/docker/Makefile.include |  2 +-
 .../dockerfiles/debian-buildroot-base.docker  | 26 +++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 tests/docker/dockerfiles/debian-buildroot-base.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index d3101afecd..74a82de48a 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -6,7 +6,7 @@ DOCKER_SUFFIX := .docker
 DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
 DOCKER_DEPRECATED_IMAGES := debian
 # we don't run tests on intermediate images (used as base by another image)
-DOCKER_PARTIAL_IMAGES := debian debian8 debian9 debian8-mxe debian-ports 
debian-sid debian-bootstrap
+DOCKER_PARTIAL_IMAGES := debian debian8 debian9 debian8-mxe debian-ports 
debian-sid debian-bootstrap debian-buildroot-base
 DOCKER_IMAGES := $(filter-out $(DOCKER_DEPRECATED_IMAGES),$(sort $(notdir 
$(basename $(wildcard $(DOCKER_FILES_DIR)/*.docker)
 DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
 # Use a global constant ccache directory to speed up repetitive builds
diff --git a/tests/docker/dockerfiles/debian-buildroot-base.docker 
b/tests/docker/dockerfiles/debian-buildroot-base.docker
new file mode 100644
index 00..c4a29abadd
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-buildroot-base.docker
@@ -0,0 +1,26 @@
+#
+# Buildroot base setup on Debian
+#
+
+FROM debian:stretch-slim
+ENV BUILDROOT_VERSION=2018.08
+
+# Duplicate deb line as deb-src
+RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> 
/etc/apt/sources.list
+
+# Install common build utilities
+RUN apt update && DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt install -y bc \
+   build-essential \
+   cpio \
+   file \
+   python \
+   unzip \
+   rsync \
+   wget
+
+# Grab the current buildroot version and unpack in /opt
+RUN mkdir -p /opt
+RUN cd /opt && wget 
https://buildroot.org/downloads/buildroot-${BUILDROOT_VERSION}.tar.gz
+RUN cd /opt && tar -xvf buildroot-${BUILDROOT_VERSION}.tar.gz
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 1/1] qemu-img: add new function to remove bitmap in image


On 9/11/18 8:56 AM, Eric Blake wrote:


+    bitmap = bdrv_find_dirty_bitmap(bs, bitmapname);
+
+    /*
+ * Dirty bitmap may not be load if the 'IN_USE' flag is set (e.g. 
the
+ * qemu thread is corrupted and the 'IN_USE' flag is not be 
cleared),

+ * so the result of bdrv_find_dirty_bitmap is null. In this case,
+ * we delete bitmap in qcow2 file directly.
+    */
+    if (!bitmap) {
+    bdrv_remove_persistent_dirty_bitmap(bs, bitmapname, &local_err);
+    if (local_err != NULL) {
+    ret = -1;
+    goto out;
+    }
+    } else {
+    if (bdrv_dirty_bitmap_get_persistance(bitmap)) {
+    bdrv_remove_persistent_dirty_bitmap(bs, bitmapname, 
&local_err);

+    if (local_err != NULL) {
+    ret = -1;
+    goto out;
+    }
+    }
+    bdrv_release_dirty_bitmap(bs, bitmap);
+    }


Why aren't you calling bdrv_block_dirty_bitmap_remove()?  In general, 


It helps if I ask my actual intended question: Why aren't you calling 
qmp_block_dirty_bitmap_remove()?


HMP commands that are mere wrappers around counterpart QMP commands are 
easier to maintain, rather than open-coding the same work in two places.




--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [RFC PATCH 4/4] tests/tcg: add nios2 architecture (NEEDS FIXES)

Now we have a docker image with a nios2 compiler we can add the bits
to build our TCG tests.

Current failures:
  testmmap - fails in check_file_fixed_eof_mmaps due to inversion of offset
  linux-test - unimplemented lseek (probably others as well)

Signed-off-by: Alex Bennée 
---
 tests/tcg/nios2/Makefile.include | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 tests/tcg/nios2/Makefile.include

diff --git a/tests/tcg/nios2/Makefile.include b/tests/tcg/nios2/Makefile.include
new file mode 100644
index 00..2ab4160435
--- /dev/null
+++ b/tests/tcg/nios2/Makefile.include
@@ -0,0 +1,2 @@
+DOCKER_IMAGE=buildroot-nios2-cross
+DOCKER_CROSS_COMPILER=nios2-linux-gcc
-- 
2.17.1

[Qemu-devel] [RFC PATCH 0/4] Add Nios II cross-compiler and enable tests/tcg

Hi,

tl;dr Nios II linux-user seems pretty broken

Following up on some mailing list queries about the state of Nios II
Thomas pointed out that buildroot could build cross-compilers for the
architecture. As a quick experiment I've enabled a docker based
buildroot and turned on tests/tcg for it.

The results are not very encouraging as both linux-test and test-mmap
fail although (impressively?) testthread passes.

The test-mmap failure looks like some sort of argument mangling
failure as arg6 of target_mmap looks negative/big and hence causes the
mmap to fail. I tried messing with the #ifdef mangling logic in
do_syscall1 but failed to get it working.

The linux-test failure seems like a missing lseek system call but I
have no doubt there are others given the bug reports on list.

This series includes a hack to min uname which we can't really merge.
This is because buildroot is focused on building system images so sets
the glibc minimum kernel version to whatever it builds for the system
image. I leave the problem of programatically tuning the
qemu_nios2_10m50_defconfig to build a general purpose glibc to whoever
wants to take this forward.

As I'm not particularly interested in this architecture I don't intend
to spend any more time on this. Given how broken linux-user is I
suspect most users of QEMU's Nios 2 emulation use softmmu. If there is
interest in fixing up linux-user then the docker and test/tcg patches
can be included when the fixups are sent to the list. I would argue
that any linux-user target should at the very least pass all of
tests/tcg/multiarch - it's not super comprehensive but it's all we
have at the moment. Perhaps we should consider deprecating the
obviously less used linux-user targets?

Alex Bennée (4):
  docker: add debian-buildroot-base
  docker: add buildroot-nios2-cross image
  linux-user/nios2: bump min uname to 4.16.0 [!HACK]
  tests/tcg: add nios2 architecture (NEEDS FIXES)

 linux-user/nios2/target_syscall.h |  2 +-
 tests/docker/Makefile.include |  6 -
 .../dockerfiles/buildroot-nios2-cross.docker  | 10 +++
 .../dockerfiles/debian-buildroot-base.docker  | 26 +++
 tests/tcg/nios2/Makefile.include  |  2 ++
 5 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 tests/docker/dockerfiles/buildroot-nios2-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-buildroot-base.docker
 create mode 100644 tests/tcg/nios2/Makefile.include

-- 
2.17.1

[Qemu-devel] [RFC PATCH 3/4] linux-user/nios2: bump min uname to 4.16.0 [!HACK]

This is to work around the limitations of the buildroot
qemu_nios2_10m50_defconfig which sets the base kernel version for
glibc.

Signed-off-by: Alex Bennée 
---
 linux-user/nios2/target_syscall.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/nios2/target_syscall.h 
b/linux-user/nios2/target_syscall.h
index ca6b7e69f6..905b80d112 100644
--- a/linux-user/nios2/target_syscall.h
+++ b/linux-user/nios2/target_syscall.h
@@ -2,7 +2,7 @@
 #define TARGET_SYSCALL_H
 
 #define UNAME_MACHINE "nios2"
-#define UNAME_MINIMUM_RELEASE "3.19.0"
+#define UNAME_MINIMUM_RELEASE "4.16.0"
 
 struct target_pt_regs {
 unsigned long  r8;/* r8-r15 Caller-saved GP registers */
-- 
2.17.1

[Qemu-devel] [RFC PATCH 2/4] docker: add buildroot-nios2-cross image

Build a buildroot toolchain for the nios2 target.

Signed-off-by: Alex Bennée 
---
 tests/docker/Makefile.include |  4 
 tests/docker/dockerfiles/buildroot-nios2-cross.docker | 10 ++
 2 files changed, 14 insertions(+)
 create mode 100644 tests/docker/dockerfiles/buildroot-nios2-cross.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 74a82de48a..a8dfde8ed5 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -120,6 +120,10 @@ docker-image-debian-riscv64-cross: docker-image-debian-sid
 docker-image-debian-powerpc-cross: docker-image-debian-sid
 docker-image-travis: NOUSER=1
 
+# Buildroot base images
+# These involve building the toolchains and can take some time
+docker-image-buildroot-nios2-cross: docker-image-debian-buildroot-base
+
 # Specialist build images, sometimes very limited tools
 docker-image-tricore-cross: docker-image-debian9
 
diff --git a/tests/docker/dockerfiles/buildroot-nios2-cross.docker 
b/tests/docker/dockerfiles/buildroot-nios2-cross.docker
new file mode 100644
index 00..e573f0fa55
--- /dev/null
+++ b/tests/docker/dockerfiles/buildroot-nios2-cross.docker
@@ -0,0 +1,10 @@
+#
+# NIOS II toolchain
+#
+FROM qemu:debian-buildroot-base
+
+RUN cd /opt/buildroot-${BUILDROOT_VERSION} && make qemu_nios2_10m50_defconfig
+RUN cd /opt/buildroot-${BUILDROOT_VERSION} && make toolchain
+
+# The toolchain is in 
/opt/buildroot-${BUILDROOT_VERSION}/output/host/bin/nios2-*
+RUN ln -s /opt/buildroot-${BUILDROOT_VERSION}/output/host/bin/nios2-* /usr/bin
-- 
2.17.1

Re: [Qemu-devel] [PATCH v1 1/1] qemu-img: add new function to remove bitmap in image