[Qemu-devel] [PATCH] lsi53c895a: convert to trace-events

2018-09-16 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
---
 hw/scsi/lsi53c895a.c | 214 +--
 hw/scsi/trace-events |  62 +++
 2 files changed, 165 insertions(+), 111 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 955ba94800..77e48ba08c 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -20,20 +20,7 @@
 #include "hw/scsi/scsi.h"
 #include "sysemu/dma.h"
 #include "qemu/log.h"
-
-//#define DEBUG_LSI
-//#define DEBUG_LSI_REG
-
-#ifdef DEBUG_LSI
-#define DPRINTF(fmt, ...) \
-do { printf("lsi_scsi: " fmt , ## __VA_ARGS__); } while (0)
-#define BADF(fmt, ...) \
-do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__); exit(1);} 
while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
-#define BADF(fmt, ...) \
-do { fprintf(stderr, "lsi_scsi: error: " fmt , ## __VA_ARGS__);} while (0)
-#endif
+#include "trace.h"
 
 static const char *names[] = {
 "SCNTL0", "SCNTL1", "SCNTL2", "SCNTL3", "SCID", "SXFER", "SDID", "GPREG",
@@ -312,7 +299,7 @@ static inline int lsi_irq_on_rsl(LSIState *s)
 
 static void lsi_soft_reset(LSIState *s)
 {
-DPRINTF("Reset\n");
+trace_lsi_reset();
 s->carry = 0;
 
 s->msg_action = 0;
@@ -473,15 +460,13 @@ static void lsi_update_irq(LSIState *s)
 level = 1;
 
 if (level != last_level) {
-DPRINTF("Update IRQ level %d dstat %02x sist %02x%02x\n",
-level, s->dstat, s->sist1, s->sist0);
+trace_lsi_update_irq(level, s->dstat, s->sist1, s->sist0);
 last_level = level;
 }
 pci_set_irq(d, level);
 
 if (!level && lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON)) {
-DPRINTF("Handled IRQs & disconnected, looking for pending "
-"processes\n");
+trace_lsi_update_irq_disconnected();
 QTAILQ_FOREACH(p, &s->queue, next) {
 if (p->pending) {
 lsi_reselect(s, p);
@@ -497,8 +482,7 @@ static void lsi_script_scsi_interrupt(LSIState *s, int 
stat0, int stat1)
 uint32_t mask0;
 uint32_t mask1;
 
-DPRINTF("SCSI Interrupt 0x%02x%02x prev 0x%02x%02x\n",
-stat1, stat0, s->sist1, s->sist0);
+trace_lsi_script_scsi_interrupt(stat1, stat0, s->sist1, s->sist0);
 s->sist0 |= stat0;
 s->sist1 |= stat1;
 /* Stop processor on fatal or unmasked interrupt.  As a special hack
@@ -516,7 +500,7 @@ static void lsi_script_scsi_interrupt(LSIState *s, int 
stat0, int stat1)
 /* Stop SCRIPTS execution and raise a DMA interrupt.  */
 static void lsi_script_dma_interrupt(LSIState *s, int stat)
 {
-DPRINTF("DMA Interrupt 0x%x prev 0x%x\n", stat, s->dstat);
+trace_lsi_script_dma_interrupt(stat, s->dstat);
 s->dstat |= stat;
 lsi_update_irq(s);
 lsi_stop_script(s);
@@ -536,9 +520,9 @@ static void lsi_bad_phase(LSIState *s, int out, int 
new_phase)
 } else {
 s->dsp = (s->scntl2 & LSI_SCNTL2_WSR ? s->pmjad2 : s->pmjad1);
 }
-DPRINTF("Data phase mismatch jump to %08x\n", s->dsp);
+trace_lsi_bad_phase_jump(s->dsp);
 } else {
-DPRINTF("Phase mismatch interrupt\n");
+trace_lsi_bad_phase_interrupt();
 lsi_script_scsi_interrupt(s, LSI_SIST0_MA, 0);
 lsi_stop_script(s);
 }
@@ -565,7 +549,7 @@ static void lsi_disconnect(LSIState *s)
 
 static void lsi_bad_selection(LSIState *s, uint32_t id)
 {
-DPRINTF("Selected absent target %d\n", id);
+trace_lsi_bad_selection(id);
 lsi_script_scsi_interrupt(s, 0, LSI_SIST1_STO);
 lsi_disconnect(s);
 }
@@ -580,7 +564,7 @@ static void lsi_do_dma(LSIState *s, int out)
 assert(s->current);
 if (!s->current->dma_len) {
 /* Wait until data is available.  */
-DPRINTF("DMA no data available\n");
+trace_lsi_do_dma_unavailable();
 return;
 }
 
@@ -600,7 +584,7 @@ static void lsi_do_dma(LSIState *s, int out)
 else if (s->sbms)
 addr |= ((uint64_t)s->sbms << 32);
 
-DPRINTF("DMA addr=0x" DMA_ADDR_FMT " len=%d\n", addr, count);
+trace_lsi_do_dma(addr, count);
 s->csbc += count;
 s->dnad += count;
 s->dbc -= count;
@@ -629,7 +613,7 @@ static void lsi_queue_command(LSIState *s)
 {
 lsi_request *p = s->current;
 
-DPRINTF("Queueing tag=0x%x\n", p->tag);
+trace_lsi_queue_command(p->tag);
 assert(s->current != NULL);
 assert(s->current->dma_len == 0);
 QTAILQ_INSERT_TAIL(&s->queue, s->current, next);
@@ -643,9 +627,9 @@ static void lsi_queue_command(LSIState *s)
 static void lsi_add_msg_byte(LSIState *s, uint8_t data)
 {
 if (s->msg_len >= LSI_MAX_MSGIN_LEN) {
-BADF("MSG IN data too long\n");
+trace_lsi_add_msg_byte_error();
 } else {
-DPRINTF("MSG IN 0x%02x\n", data);
+trace_lsi_add_msg_byte(data);
 s->msg[s->msg_len++] = data;
 }
 }
@@ -665,7 +649,7 @@ static void lsi_reselect(LSIState *s, lsi_request *p)
 if (!(s->dcntl & LSI_DCNTL_COM)) {
 s->sfbr = 1 << (id & 0x7);
 

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

2018-09-16 Thread Peter Xu
On Fri, Sep 14, 2018 at 01:27:00PM -0500, Brijesh Singh wrote:
> Emulate the interrupt remapping support when guest virtual APIC is
> not enabled.
> 
> For more info Refer: AMD IOMMU spec Rev 3.0 - section 2.2.5.1
> 
> When VAPIC is not enabled, it uses interrupt remapping as defined in
> Table 20 and Figure 15 from IOMMU spec.

(feel free to cc me in your next post)

> 
> 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  | 189 
> ++-
>  hw/i386/amd_iommu.h  |  46 -
>  hw/i386/trace-events |   7 ++
>  3 files changed, 240 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index b15962b..9c8e4de 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[] = {
> @@ -1029,17 +1031,144 @@ 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;

(I'll have similar endianess question like previous patch, but I'll
 stop looking at those since I'll need to know whether you plan to
 support that first...)

> +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 int amdvi_int_remap_legacy(AMDVIState *iommu,
> +  MSIMessage *origin,
> +  MSIMessage *translated,
> +  uint64_t *dte,
> +  X86IOMMUIrq *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");

Note that recently QEMU introduced error_report_once().  Feel free to
switch to that if you want.  Many of the VT-d emulation code switched
to that to make sure errors like this will be dumped at least once and
we're also free from Dos attack.  This should at least apply to all of
your below trace_amdvi_ir_target_abort() calls, even some other traces.

> +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,
> + X86IOMMUIrq *irq,
> + uint16_t sid)
> +{
> +uint8_t int_ctl;
> +
> +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;
> +}
> +
> +return amdvi_int_remap_legacy(iommu, origin, translated, dte, irq, sid);
> +}
> +
> +static bool amdvi_validate_int_reamp(AMDVIState *s, uint64_t *dte)

s/reamp/remap/

> +{
> +/* Check if IR is enabled in DTE */
> +if (!(dte[2] & AMDVI_IR_REMAP_ENABLE)) {
> +return false;
> +}

[Qemu-devel] [PATCH v2 0/2] hw/vfio/display: add ramfb support

2018-09-16 Thread Gerd Hoffmann
So we have a boot display when using a vgpu as primary display.

Gerd Hoffmann (2):
  stubs: add ramfb
  hw/vfio/display: add ramfb support

 hw/vfio/pci.h |  1 +
 include/hw/vfio/vfio-common.h |  2 ++
 hw/vfio/display.c | 12 
 hw/vfio/pci.c | 25 +
 stubs/ramfb.c | 13 +
 stubs/Makefile.objs   |  1 +
 6 files changed, 54 insertions(+)
 create mode 100644 stubs/ramfb.c

-- 
2.9.3




[Qemu-devel] [PATCH v2 1/2] stubs: add ramfb

2018-09-16 Thread Gerd Hoffmann
Needed to make sure code using ramfb (vfio) compiles properly even on
platforms without fw_cfg (and therefore no ramfb) support.

Signed-off-by: Gerd Hoffmann 
---
 stubs/ramfb.c   | 13 +
 stubs/Makefile.objs |  1 +
 2 files changed, 14 insertions(+)
 create mode 100644 stubs/ramfb.c

diff --git a/stubs/ramfb.c b/stubs/ramfb.c
new file mode 100644
index 00..48143f3354
--- /dev/null
+++ b/stubs/ramfb.c
@@ -0,0 +1,13 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/display/ramfb.h"
+
+void ramfb_display_update(QemuConsole *con, RAMFBState *s)
+{
+}
+
+RAMFBState *ramfb_setup(Error **errp)
+{
+error_setg(errp, "ramfb support not available");
+return NULL;
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 53d3f32cb2..5dd0aeeec6 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -43,3 +43,4 @@ stub-obj-y += xen-common.o
 stub-obj-y += xen-hvm.o
 stub-obj-y += pci-host-piix.o
 stub-obj-y += ram-block.o
+stub-obj-y += ramfb.o
-- 
2.9.3




[Qemu-devel] [PATCH v2 2/2] hw/vfio/display: add ramfb support

2018-09-16 Thread Gerd Hoffmann
So we have a boot display when using a vgpu as primary display.

ramfb depends on a fw_cfg file.  fw_cfg files can not be added and
removed at runtime, therefore a ramfb-enabled vfio device can't be
hotplugged.

Add a nohotplug variant of the vfio-pci device (as child class).  Add
the ramfb property to the nohotplug variant only.  So to enable the vgpu
display with boot support use this:

  -device vfio-pci-nohotplug,display=on,ramfb=on,sysfsdev=...

Signed-off-by: Gerd Hoffmann 
---
 hw/vfio/pci.h |  1 +
 include/hw/vfio/vfio-common.h |  2 ++
 hw/vfio/display.c | 12 
 hw/vfio/pci.c | 25 +
 4 files changed, 40 insertions(+)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 52b065421a..904e286586 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -149,6 +149,7 @@ typedef struct VFIOPCIDevice {
 #define VFIO_FEATURE_ENABLE_IGD_OPREGION \
 (1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
 OnOffAuto display;
+bool enable_ramfb;
 int32_t bootindex;
 uint32_t igd_gms;
 OffAutoPCIBAR msix_relo;
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 821def0565..0d85a0a6f8 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -26,6 +26,7 @@
 #include "qemu/queue.h"
 #include "qemu/notify.h"
 #include "ui/console.h"
+#include "hw/display/ramfb.h"
 #ifdef CONFIG_LINUX
 #include 
 #endif
@@ -146,6 +147,7 @@ typedef struct VFIODMABuf {
 
 typedef struct VFIODisplay {
 QemuConsole *con;
+RAMFBState *ramfb;
 struct {
 VFIORegion buffer;
 DisplaySurface *surface;
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 59c0e5d1d7..dead30e626 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -124,6 +124,9 @@ static void vfio_display_dmabuf_update(void *opaque)
 
 primary = vfio_display_get_dmabuf(vdev, DRM_PLANE_TYPE_PRIMARY);
 if (primary == NULL) {
+if (dpy->ramfb) {
+ramfb_display_update(dpy->con, dpy->ramfb);
+}
 return;
 }
 
@@ -181,6 +184,9 @@ static int vfio_display_dmabuf_init(VFIOPCIDevice *vdev, 
Error **errp)
 vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
   &vfio_display_dmabuf_ops,
   vdev);
+if (vdev->enable_ramfb) {
+vdev->dpy->ramfb = ramfb_setup(errp);
+}
 return 0;
 }
 
@@ -228,6 +234,9 @@ static void vfio_display_region_update(void *opaque)
 return;
 }
 if (!plane.drm_format || !plane.size) {
+if (dpy->ramfb) {
+ramfb_display_update(dpy->con, dpy->ramfb);
+}
 return;
 }
 format = qemu_drm_format_to_pixman(plane.drm_format);
@@ -300,6 +309,9 @@ static int vfio_display_region_init(VFIOPCIDevice *vdev, 
Error **errp)
 vdev->dpy->con = graphic_console_init(DEVICE(vdev), 0,
   &vfio_display_region_ops,
   vdev);
+if (vdev->enable_ramfb) {
+vdev->dpy->ramfb = ramfb_setup(errp);
+}
 return 0;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 866f0deeb7..a0047f4942 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3067,6 +3067,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 goto out_teardown;
 }
 }
+if (vdev->enable_ramfb && vdev->dpy == NULL) {
+error_setg(errp, "ramfb=on requires display=on");
+goto out_teardown;
+}
 
 vfio_register_err_notifier(vdev);
 vfio_register_req_notifier(vdev);
@@ -3258,9 +3262,30 @@ static const TypeInfo vfio_pci_dev_info = {
 },
 };
 
+static Property vfio_pci_dev_nohotplug_properties[] = {
+DEFINE_PROP_BOOL("ramfb", VFIOPCIDevice, enable_ramfb, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vfio_pci_nohotplug_dev_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = vfio_pci_dev_nohotplug_properties;
+dc->hotpluggable = false;
+}
+
+static const TypeInfo vfio_pci_nohotplug_dev_info = {
+.name = "vfio-pci-nohotplug",
+.parent = "vfio-pci",
+.instance_size = sizeof(VFIOPCIDevice),
+.class_init = vfio_pci_nohotplug_dev_class_init,
+};
+
 static void register_vfio_pci_dev_type(void)
 {
 type_register_static(&vfio_pci_dev_info);
+type_register_static(&vfio_pci_nohotplug_dev_info);
 }
 
 type_init(register_vfio_pci_dev_type)
-- 
2.9.3




Re: [Qemu-devel] [PATCH v2] vnc: call sasl_server_init() only when required

2018-09-16 Thread Gerd Hoffmann
On Fri, Sep 07, 2018 at 10:36:34AM +0400, Marc-André Lureau wrote:
> VNC server is calling sasl_server_init() during startup of QEMU, even
> if SASL auth has not been enabled.
> 
> This may create undesirable warnings like "Could not find keytab file:
> /etc/qemu/krb5.tab" when the user didn't configure SASL on host and
> started VNC server.
> 
> Instead, only initialize SASL when needed. Note that HMP/QMP "change
> vnc" calls vnc_display_open() again, which will initialize SASL if
> needed.
> 
> Fix assignment in if condition, while touching this code.
> 
> Related to:
> https://bugzilla.redhat.com/show_bug.cgi?id=1609327

Added to ui queue.

thanks,
  Gerd




Re: [Qemu-devel] Overcommiting cpu results in all vms offline

2018-09-16 Thread Jack Wang
Stefan Priebe - Profihost AG  于2018年9月16日周日 下午3:31写道:
>
> Hello,
>
> while overcommiting cpu I had several situations where all vms gone offline 
> while two vms saturated all cores.
>
> I believed all vms would stay online but would just not be able to use all 
> their cores?
>
> My original idea was to automate live migration on high host load to move vms 
> to another node but that makes only sense if all vms stay online.
>
> Is this expected? Anything special needed to archive this?
>
> Greets,
> Stefan
>
Hi, Stefan,

Do you have any logs when all VMs go offline?
Maybe OOMkiller play a role there?

Regards,
Jack



Re: [Qemu-devel] virtio-net sporadic error with QNX 7.0 guest: virtio-net ctrl missing headers

2018-09-16 Thread Claudio
Hello Michael,

On 09/12/2018 07:26 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 12, 2018 at 07:12:58PM +0200, Claudio wrote:
>> Hi Michael,
>>
>> On 09/12/2018 05:31 PM, Michael S. Tsirkin wrote:
>>> On Wed, Sep 12, 2018 at 05:16:38PM +0200, Claudio wrote:
 Thank you both for your responses,

 and ciao Paolo,

 On 09/12/2018 02:37 PM, Michael S. Tsirkin wrote:
> On Wed, Sep 12, 2018 at 10:01:34AM +0200, Claudio wrote:
>> Hello Michael, Jason and all,
>>
>> I am currently using latest mainline QEMU on x86_64 to run a QNX 7 guest.
>>
>> QNX 7 is not free software anymore unfortunately, with the
>> the last open source versions in the 6.x range.
>>
>> I am using the official virtio-net guest driver from QNX 7.
>>
>> During initialization I sporadically get this error message:
>>
>> $ qemu-system-x86_64 -machine pc,accel=kvm,kernel_irqchip=on -smp 2 -m 
>> 2048 -display none -nodefconfig -nodefaults -chardev 
>> stdio,mux=on,id=char0 -serial chardev:char0 -monitor none -mon 
>> chardev=char0,mode=readline -netdev 
>> user,id=user0,hostfwd=udp::9004-:9004 -device virtio-net,netdev=user0 
>> qnx.img
>>
>> virtio-net ctrl missing headers
>
> This means a control buffer is sent either without the input element
> or without an output element, or with a single byte output element.
>
>> and following that my host->guest UDP port forwarding does not work, 
>> that is,
>> the qemu process shows up as listening on the interface, but no packets 
>> appear in the guest.
>>
>> This error during initialization does not appear every time I launch 
>> QEMU.
>> It appears to be more or less random.
>>
>> Whenever the error does not appear, the interface works as expected, and 
>> port forwarding works.
>>
>> Latest commit is 
>>
>> 19b599f7664b ("Merge remote-tracking branch 
>> 'remotes/armbru/tags/pull-error-2018-08-27-v2'")
>>
>> Thanks a lot for any advice!
>>
>> Ciao,
>>
>> Claudio
>
> If it's easy to reproduce, you can try printing out all commands.
> E.g.:
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index f154756e85..34251273a9 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -987,6 +987,10 @@ static void virtio_net_handle_ctrl(VirtIODevice 
> *vdev, VirtQueue *vq)
>  if (!elem) {
>  break;
>  }
> +fprintf(stderr, "%s: in %d/%d, out %d/%d\n",
> +elem->in_num, iov_size(elem->in_sg, elem->in_num),
> +elem->out_num, iov_size(elem->out_sg, elem->out_num));
> +
>  if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
>  iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
>  virtio_error(vdev, "virtio-net ctrl missing headers");
> @@ -1014,6 +1018,9 @@ static void virtio_net_handle_ctrl(VirtIODevice 
> *vdev, VirtQueue *vq)
>  } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
>  status = virtio_net_handle_offloads(n, ctrl.cmd, iov, 
> iov_cnt);
>  }
> +fprintf(stderr, "%s: class 0x%x cmd 0x%x cnt %d status 0x%x\n",
> +ctrl.class, ctrl.cmd, iov_cnt, status);
> +
>  
>  s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, 
> sizeof(status));
>  assert(s == sizeof(status));
>
>
> You can also try replacing virtio_error with fprintf - that will
> avoid wedging the device on an error and let you proceed
> with debugging.
>

 I managed to automate the detection of the error, and it does appear to be 
 a regression from my point of view.

 I did a git bisect, and you can see the result and log as follows:

 >From 4a3f03ba8dbf53fce36d0c1dd5d0cc0f340fe5f3 Mon Sep 17 00:00:00 2001
 >   
 From: Paolo Bonzini   
   
 Date: Wed, 11 Jan 2017 09:38:15 +0100  
   
 Subject: [PATCH] virtio-net: enable ioeventfd even if vhost=off
   

   
 virtio-net-pci does not enable ioeventfd for historical reasons (and   
   
 nobody ever checked whether it should be revisited).  Note that other  
   
 backends do enable ioeventfd for virtio-net.   
   

   
 However, it has a major effect on performance.  On Windows, throughput is  
   
 _multiplied_ by 2 or 3 on TCP_STREAM (on small packe

[Qemu-devel] Qemu shutdown VMs with the error in transmitting console data

2018-09-16 Thread Naruto Nguyen
Hello everyone,

Recently when I reboot my guest installed kernel 4.4.114, qemu-kvm
throws the error

qemu-kvm: hw/char/serial.c:232: serial_xmit: Assertion `!(s->lsr &
0x40)' failed.
shutting down

then qemu-kvm powers off the VM, I can meet the issue after a lot of
time to reboot the VM. The issue looks like to be reproduced more
easily by increasing the loglevel to make kernel throws more log to
the console. It seems I did not meet the issue with guest installed
kernel 3.0. There is another thread of Redhat about this bug but looks
like they cannot reproduce the issue and just provide the potential
fix https://bugzilla.redhat.com/show_bug.cgi?id=1481701.

I used the same qemu version with Redhat thread, qemu 2.6
qemu-kvm-rhev-2.6.0-28.el7_3.9.x86_64. My guest xml setup is to
redirect the console to file. I used the same config between guest
kernel 3.0 and 4.4. Could you please help me to understand more about
this issue:

- In what conditions can make qemu-kvm trigger this issue like: guest
console hangs or guest output to console too much?
- Why I just see the issue on guest installed kernel 4.4, not on kernel 3.0?

Thanks a lot,
Brs,
Naruto



Re: [Qemu-devel] Qemu shutdown VMs with the error in transmitting console data

2018-09-16 Thread Naruto Nguyen
Hi again,

Could be there any workaround to avoid VM  shutdown?

Thanks,
Brs,
Bao
On Sun, 16 Sep 2018 at 16:01, Naruto Nguyen  wrote:
>
> Hello everyone,
>
> Recently when I reboot my guest installed kernel 4.4.114, qemu-kvm
> throws the error
>
> qemu-kvm: hw/char/serial.c:232: serial_xmit: Assertion `!(s->lsr &
> 0x40)' failed.
> shutting down
>
> then qemu-kvm powers off the VM, I can meet the issue after a lot of
> time to reboot the VM. The issue looks like to be reproduced more
> easily by increasing the loglevel to make kernel throws more log to
> the console. It seems I did not meet the issue with guest installed
> kernel 3.0. There is another thread of Redhat about this bug but looks
> like they cannot reproduce the issue and just provide the potential
> fix https://bugzilla.redhat.com/show_bug.cgi?id=1481701.
>
> I used the same qemu version with Redhat thread, qemu 2.6
> qemu-kvm-rhev-2.6.0-28.el7_3.9.x86_64. My guest xml setup is to
> redirect the console to file. I used the same config between guest
> kernel 3.0 and 4.4. Could you please help me to understand more about
> this issue:
>
> - In what conditions can make qemu-kvm trigger this issue like: guest
> console hangs or guest output to console too much?
> - Why I just see the issue on guest installed kernel 4.4, not on kernel 3.0?
>
> Thanks a lot,
> Brs,
> Naruto



[Qemu-devel] [QEMU PATCH v2 1/2] i386: Compile CPUX86State xsave_buf only when support KVM or HVF

2018-09-16 Thread Liran Alon
While at it, also rename var to indicate it is not used only in KVM.

Reviewed-by: Nikita Leshchenko 
Reviewed-by: Patrick Colp 
Reviewed-by: Mihai Carabas 
Signed-off-by: Liran Alon 
---
 target/i386/cpu.h | 4 +++-
 target/i386/hvf/README.md | 2 +-
 target/i386/hvf/hvf.c | 2 +-
 target/i386/hvf/x86hvf.c  | 4 ++--
 target/i386/kvm.c | 6 +++---
 5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index b572a8e4aa41..6e4c2b02f947 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1327,7 +1327,9 @@ typedef struct CPUX86State {
 bool tsc_valid;
 int64_t tsc_khz;
 int64_t user_tsc_khz; /* for sanity check only */
-void *kvm_xsave_buf;
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+void *xsave_buf;
+#endif
 #if defined(CONFIG_HVF)
 HVFX86EmulatorState *hvf_emul;
 #endif
diff --git a/target/i386/hvf/README.md b/target/i386/hvf/README.md
index 0d27a0d52b58..2d33477aca50 100644
--- a/target/i386/hvf/README.md
+++ b/target/i386/hvf/README.md
@@ -2,6 +2,6 @@
 
 These sources (and ../hvf-all.c) are adapted from Veertu Inc's vdhh (Veertu 
Desktop Hosted Hypervisor) (last known location: 
https://github.com/veertuinc/vdhh) with some minor changes, the most 
significant of which were:
 
-1. Adapt to our current QEMU's `CPUState` structure and `address_space_rw` 
API; many struct members have been moved around (emulated x86 state, 
kvm_xsave_buf) due to historical differences + QEMU needing to handle more 
emulation targets.
+1. Adapt to our current QEMU's `CPUState` structure and `address_space_rw` 
API; many struct members have been moved around (emulated x86 state, xsave_buf) 
due to historical differences + QEMU needing to handle more emulation targets.
 2. Removal of `apic_page` and hyperv-related functionality.
 3. More relaxed use of `qemu_mutex_lock_iothread`.
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index df69e6d0a7af..5db167df98e6 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -587,7 +587,7 @@ int hvf_init_vcpu(CPUState *cpu)
 hvf_reset_vcpu(cpu);
 
 x86cpu = X86_CPU(cpu);
-x86cpu->env.kvm_xsave_buf = qemu_memalign(4096, 4096);
+x86cpu->env.xsave_buf = qemu_memalign(4096, 4096);
 
 hv_vcpu_enable_native_msr(cpu->hvf_fd, MSR_STAR, 1);
 hv_vcpu_enable_native_msr(cpu->hvf_fd, MSR_LSTAR, 1);
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index 6c88939b968b..df8e946fbcde 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -75,7 +75,7 @@ void hvf_put_xsave(CPUState *cpu_state)
 
 struct X86XSaveArea *xsave;
 
-xsave = X86_CPU(cpu_state)->env.kvm_xsave_buf;
+xsave = X86_CPU(cpu_state)->env.xsave_buf;
 
 x86_cpu_xsave_all_areas(X86_CPU(cpu_state), xsave);
 
@@ -163,7 +163,7 @@ void hvf_get_xsave(CPUState *cpu_state)
 {
 struct X86XSaveArea *xsave;
 
-xsave = X86_CPU(cpu_state)->env.kvm_xsave_buf;
+xsave = X86_CPU(cpu_state)->env.xsave_buf;
 
 if (hv_vcpu_read_fpstate(cpu_state->hvf_fd, (void*)xsave, 4096)) {
 abort();
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0b2a07d3a47b..c1cd8c461fe4 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1189,7 +1189,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 
 if (has_xsave) {
-env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
+env->xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
 }
 cpu->kvm_msr_buf = g_malloc0(MSR_BUF_SIZE);
 
@@ -1639,7 +1639,7 @@ ASSERT_OFFSET(XSAVE_PKRU, pkru_state);
 static int kvm_put_xsave(X86CPU *cpu)
 {
 CPUX86State *env = &cpu->env;
-X86XSaveArea *xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->xsave_buf;
 
 if (!has_xsave) {
 return kvm_put_fpu(cpu);
@@ -2081,7 +2081,7 @@ static int kvm_get_fpu(X86CPU *cpu)
 static int kvm_get_xsave(X86CPU *cpu)
 {
 CPUX86State *env = &cpu->env;
-X86XSaveArea *xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->xsave_buf;
 int ret;
 
 if (!has_xsave) {
-- 
2.16.1




[Qemu-devel] [QEMU PATCH v2 2/2] KVM: i386: Add support for save and restore nested state

2018-09-16 Thread Liran Alon
Kernel commit 8fcc4b5923af ("kvm: nVMX: Introduce KVM_CAP_NESTED_STATE")
introduced new IOCTLs to extract and restore KVM internal state used to
run a VM that is in VMX operation.

Utilize these IOCTLs to add support of migration of VMs which are
running nested hypervisors.

Reviewed-by: Nikita Leshchenko 
Reviewed-by: Patrick Colp 
Reviewed-by: Mihai Carabas 
Signed-off-by: Liran Alon 
---
 accel/kvm/kvm-all.c   | 15 +++
 include/sysemu/kvm.h  |  1 +
 target/i386/cpu.h |  2 ++
 target/i386/kvm.c | 58 
 target/i386/machine.c | 73 +++
 5 files changed, 149 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index de12f78eb8e4..fe6377ce9bcc 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -87,6 +87,7 @@ struct KVMState
 #ifdef KVM_CAP_SET_GUEST_DEBUG
 struct kvm_sw_breakpoint_head kvm_sw_breakpoints;
 #endif
+uint32_t max_nested_state_len;
 int many_ioeventfds;
 int intx_set_mask;
 bool sync_mmu;
@@ -1628,6 +1629,15 @@ static int kvm_init(MachineState *ms)
 s->debugregs = kvm_check_extension(s, KVM_CAP_DEBUGREGS);
 #endif
 
+ret = kvm_check_extension(s, KVM_CAP_NESTED_STATE);
+if (ret < 0) {
+fprintf(stderr,
+"kvm failed to get max size of nested state (%d)",
+ret);
+goto err;
+}
+s->max_nested_state_len = (uint32_t)ret;
+
 #ifdef KVM_CAP_IRQ_ROUTING
 kvm_direct_msi_allowed = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 0);
 #endif
@@ -2187,6 +2197,11 @@ int kvm_has_debugregs(void)
 return kvm_state->debugregs;
 }
 
+uint32_t kvm_max_nested_state_length(void)
+{
+return kvm_state->max_nested_state_len;
+}
+
 int kvm_has_many_ioeventfds(void)
 {
 if (!kvm_enabled()) {
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e06786..352c7fd4e3d2 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -210,6 +210,7 @@ bool kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
 int kvm_has_robust_singlestep(void);
 int kvm_has_debugregs(void);
+uint32_t kvm_max_nested_state_length(void);
 int kvm_has_pit_state2(void);
 int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 6e4c2b02f947..3b97b5b280f0 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1330,6 +1330,8 @@ typedef struct CPUX86State {
 #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
 void *xsave_buf;
 #endif
+struct kvm_nested_state *nested_state;
+uint32_t nested_state_len;   /* needed for migration */
 #if defined(CONFIG_HVF)
 HVFX86EmulatorState *hvf_emul;
 #endif
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index c1cd8c461fe4..aeb55b5ed6f5 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1191,6 +1191,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
 if (has_xsave) {
 env->xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
 }
+
+env->nested_state_len = kvm_max_nested_state_length();
+if (env->nested_state_len > 0) {
+uint32_t min_nested_state_len =
+offsetof(struct kvm_nested_state, size) + sizeof(uint32_t);
+
+/*
+ * Verify nested state length cover at least the size
+ * field of struct kvm_nested_state
+ */
+assert(env->nested_state_len >= min_nested_state_len);
+
+env->nested_state = g_malloc0(env->nested_state_len);
+env->nested_state->size = env->nested_state_len;
+}
+
 cpu->kvm_msr_buf = g_malloc0(MSR_BUF_SIZE);
 
 if (!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_RDTSCP)) {
@@ -2867,6 +2883,39 @@ static int kvm_get_debugregs(X86CPU *cpu)
 return 0;
 }
 
+static int kvm_put_nested_state(X86CPU *cpu)
+{
+CPUX86State *env = &cpu->env;
+
+if (kvm_max_nested_state_length() == 0) {
+return 0;
+}
+
+assert(env->nested_state->size <= env->nested_state_len);
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_NESTED_STATE, env->nested_state);
+}
+
+static int kvm_get_nested_state(X86CPU *cpu)
+{
+CPUX86State *env = &cpu->env;
+
+if (kvm_max_nested_state_length() == 0) {
+return 0;
+}
+
+
+/*
+ * It is possible that migration restored a smaller size into
+ * nested_state->size than what our kernel support.
+ * We preserve migration origin nested_state->size for
+ * call to KVM_SET_NESTED_STATE but wish that our next call
+ * to KVM_GET_NESTED_STATE will use max size our kernel support.
+ */
+env->nested_state->size = env->nested_state_len;
+
+return kvm_vcpu_ioctl(CPU(cpu), KVM_GET_NESTED_STATE, env->nested_state);
+}
+
 int kvm_arch_put_registers(CPUState *cpu, int level)
 {
 X86CPU *x86_cpu = X86_CPU(cpu);
@@ -2874,6 +2923,11 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
 
 assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu));
 
+ret = kvm_put_nested_state(x86_cpu);
+if 

[Qemu-devel] [QEMU PATCH v2 0/2]: KVM: i386: Add support for save and restore nested state

2018-09-16 Thread Liran Alon
Hi,

This series aims to add support for QEMU to be able to migrate VMs that
are running nested hypervisors. In order to do so, it utilizes the new
IOCTLs introduced in KVM commit 8fcc4b5923af ("kvm: nVMX: Introduce
KVM_CAP_NESTED_STATE") which were created for this purpose.

1st patch is not really related to the goal of the patch series. It just
makes CPUX86State->xsave_buf to be compiled only when needed (When
compiling with KVM or HVF CPU accelerator).

2nd patch adds the support to migrate VMs that are running nested
hypervisors.

Regards,
-Liran

v1->v2 Changes:
* Renamed kvm_nested_state_length() to kvm_max_nested_state_length()
to better indicate it represents the max nested state size that can
be returned from kernel.
* Added error_report() calls to nested_state_post_load() to make
failures in migration easier to diagnose.
* Fixed support of migrating with various nested_state buffer sizes.
The following scenarios were tested:
(a) src and dest have same nested state size.
==> Migration succeeds.
(b) src don't have nested state while dest do.
==> Migration succeed and src don't send it's nested state.
(c) src have nested state while dest don't.
==> Migration fails as it cannot restore nested state.
(d) dest have bigger max nested state size than src
==> Migration succeeds.
(e) dest have smaller max nested state size than src but enough to store it's 
saved nested state
==> Migration succeeds
(f) dest have smaller max nested state size than src but not enough to store 
it's saved nested state
==> Migration fails




[Qemu-devel] Overcommiting cpu results in all vms offline

2018-09-16 Thread Stefan Priebe - Profihost AG
Hello,

while overcommiting cpu I had several situations where all vms gone offline 
while two vms saturated all cores.

I believed all vms would stay online but would just not be able to use all 
their cores?

My original idea was to automate live migration on high host load to move vms 
to another node but that makes only sense if all vms stay online.

Is this expected? Anything special needed to archive this?

Greets,
Stefan



Re: [Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

2018-09-16 Thread Richard Henderson
On 9/15/18 6:32 PM, Peter Maydell wrote:
> In the current scheme of doing things I'd look for whether
> we could say that some more generic thing implied SWP
> rather than setting it in a lot of initfns (eg v5-but-not-v7VE?),
> but maybe the later patches make that a bad approach
> (haven't looked at the meat of this series).

Perhaps.  But indeed this goes back to the main question
posed in the cover letter.

> We want to arrange to have SWP work anyway on linux-user,
> I think, since the kernel will typically trap-and-emulate
> it assuming it was built with CONFIG_SWP_EMULATE. (I don't
> know if those kernels will advertise swp in the hwcaps,
> but I guess not.)

Ah, I did not know about SWP_EMULATE.  It appears to be
specific to armv7+ (though we don't support the pre-v4
cpus for which it might otherwise be relevant).

It does appear that HWCAP_SWP is advertised anyway:

mm/proc-v7.S:   .long   HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT


r~




Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

2018-09-16 Thread Fredrik Noring
Many thanks for your review, Maciej,

>  I have been more thorough on this occasion, and I do hope I have caught 
> everything.  See the notes below, in addition to what the others wrote.  
> 
>  Please apply to v3 accordingly; I started writing this before you sent 
> that version.

Sure, next version submitted as a patch series will be a lot better, thanks!

>  I think this has to be (CPU_MIPS3 | INSN_R5900) really.
> 
>  While the CPU does support MIPS IV MOVN, MOVZ and PREF instructions, 
> that's all it has from that ISA.  Even the Tx79, which was supposed to 
> have a regular IEEE-754 FPU, did not have the extra FP condition bits, 
> MOVN.fmt, MOVZ.fmt or any of the MOVF* or MOVT* instructions, indexed 
> load/store instructions, multiply-accumulate instructions, etc. defined.
> 
>  So I think the R5900 has to be treated as a MIPS III implementation, with 
> some aberrations.  I don't expect it to be a big deal to add INSN_R5900 
> qualification to MOVN, MOVZ and PREF instruction emulation code right 
> away.

Trivial to add, in fact. :)

>  Then presumably you are going to add emulation for all the missing MIPS 
> III instructions to the Linux kernel eventually, to match the MIPS III 
> Linux user ABI?  Apart from LLD and SCD discussed previously this will 
> have to include DDIV, DDIVU, DMULT and DMULTU.

That sounds reasonable!

>  Eventually you'll have to remove all these instructions (plus LL and SC) 
> from the system emulation mode.  In fact I think it would make sense to do 
> that right away, because I believe it will be a reasonably simple update. 
> However if it turns out to be a can of worms after all, then I think we 
> can defer it, because we do not have a bare-metal environment ready for 
> this CPU anyway (or do we?).

Indeed, very simple. I added a new check function check_insn_opc_user_only,
similar to check_insn_opc_removed, conditional on USER_ONLY and so a simple

check_insn_opc_user_only(ctx, INSN_R5900);

has to be added for these OPC cases. To test this I tried to build GCC with
the target mips64r5900el but it unfortunately didn't work as GCC failed to
compile itself:

../sysdeps/mips/mips64/mul_1.S:49: Error: opcode not supported on
this processor: r5900 (mips3) `dmultu $8,$7'

>  I think there is no point in executing the multiplication twice if `rd != 
> 0' and also we want to continue trapping on `sa != 0' for the non-Vr54xx 
> case, so how about:
> 
> if (sa) {
> check_insn(ctx, INSN_VR54XX);
> op1 = MASK_MUL_VR54XX(ctx->opcode);
> gen_mul_vr54xx(ctx, op1, rd, rs, rt);
> } else if (rd && (ctx->insn_flags & INSN_R5900)) {
> gen_mul_r5900(ctx, op1, rd, rs, rt);
> } else {
> gen_muldiv(ctx, op1, rd & 3, rs, rt);
> }
> 
> ?

Agreed, that's better. The point was to reuse a bit of code, but now I've
folded all of it into gen_mul_r5900. There are also R5900 specific MULT1 and
MULTU1 instructions which might use it, so I folded the rd != 0 test into it
as well.

> > +/* No L2 cache, icache size 32k, dcache size 32k, uncached 
> > coherency. */
> > +.CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << 
> > CP0C0_K0),
> 
>  Why ICE set but DCE clear?  I guess this corresponds to the incorrect L2 
> cache reference as bit #17 is SC on the original R4000.  That reference 
> has to go, obviously, as there's no L2 cache indication in the R5900's 
> Config register.
> 
>  As to ICE and DCE their reset defaults are both 0, so we might as well 
> use that, as we don't emulate caches anyway.  If it turns out to matter to 
> any software, then we'll have to provide a more correct Config register 
> emulation as its writable bits are processor-specific.

Agreed, thanks for finding this!

> > +/* Note: Config1 is only used internally, the R5900 has only 
> > Config0. */
> > +.CP0_Status_rw_bitmask = 0xF4C79C1F,
> 
>  Maybe swap the two lines so that the Config1 register reference does not 
> confusingly seem to describe the Status r/w bitmask?

Done.

> > +#ifdef CONFIG_USER_ONLY
> > +   /*
> > +* R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
> > +* emulation. For user-only, qemu is the kernel, so we emulate the traps
> > +* by simply emulating the instructions directly.
> > +*/
> 
>  Please use spaces rather than tabs for indentation, as per QEMU's coding 
> standard.

Done.

> > +.SEGBITS = 19,
> 
>  This has to be 32, as with any MIPS CPU that implements 32-bit virtual 
> addressing only.  Specifically EntryHi register's VPN2 field goes up to 
> bit #31, which is (SEGBITS - 1).
> 
> > +.PABITS = 20,
> 
>  And this has to be 32 as well, reflecting EntryLo0/1 registers' PFN 
> fields going up to bit #25.  This corresponds to bit #31 on the external 
> address bus, which is (PABITS - 1).

Right. I was unaware of these details, thanks!

Fredrik



Re: [Qemu-devel] [PATCH 01/13] target/arm: Add ARM_FEATURE_SWP

2018-09-16 Thread Peter Maydell
On 16 September 2018 at 16:53, Richard Henderson
 wrote:
> On 9/15/18 6:32 PM, Peter Maydell wrote:
>> We want to arrange to have SWP work anyway on linux-user,
>> I think, since the kernel will typically trap-and-emulate
>> it assuming it was built with CONFIG_SWP_EMULATE. (I don't
>> know if those kernels will advertise swp in the hwcaps,
>> but I guess not.)
>
> Ah, I did not know about SWP_EMULATE.  It appears to be
> specific to armv7+ (though we don't support the pre-v4
> cpus for which it might otherwise be relevant).

Yes, it's intended to allow older userspace binaries to continue
to work on newer CPUs without SWP, not to try to run new binaries
on older CPUs. (Anything compiled for a CPU new enough for SWP
probably uses other newer insns that some pre-v4 CPU doesn't have
anyway.)

> It does appear that HWCAP_SWP is advertised anyway:
>
> mm/proc-v7.S:   .long   HWCAP_SWP | HWCAP_HALF | HWCAP_THUMB | HWCAP_FAST_MULT

Interesting. I might ask some random kernel person what the semantics
of hwcap are -- is it "will work even if a terrible plan" or "it makes
sense to use this"?

thanks
-- PMM



[Qemu-devel] [PATCH v4 0/8] target/mips: Support R5900 GCC programs in user mode

2018-09-16 Thread Fredrik Noring
The primary purpose of this change is to support programs compiled by
GCC for the R5900 target and thereby run R5900 Linux distributions, for
example Gentoo. In particular, this avoids issues with cross compilation.

This change has been tested with Gentoo compiled for R5900, including
native compilation of several packages under QEMU.

The R5900 implements the 64-bit MIPS III instruction set except DMULT,
DMULTU, DDIV, DDIVU, LL, SC, LLD and SCD. The MIPS IV instructions MOVN,
MOVZ and PREF are implemented. It has the R5900 specific three-operand
instructions MADD, MADDU, MULT and MULTU as well as pipeline 1 versions
MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and
MTLO1. A set of 93 128-bit multimedia instructions specific to the
R5900 is also implemented.

The Toshiba TX System RISC TX79 Core Architecture manual describes the
R5900 processor:

http://www.lukasz.dk/files/tx79architecture.pdf

Fredrik Noring (8):
  target/mips: Define R5900 instructions and CPU preprocessor constants
  target/mips: Support R5900 specific three-operand MULT and MULTU
  target/mips: Support R5900 instructions MOVN, MOVZ and PREF from MIPS IV
  target/mips: Add function to signal RI exception unless user only
  target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only
  target/mips: Define the R5900 CPU
  linux-user/mips: Recognise the R5900 CPU model
  elf: Toshiba/Sony rather than MIPS are the implementors of the R5900

 include/elf.h|  2 +-
 linux-user/mips/target_elf.h |  3 ++
 target/mips/mips-defs.h  |  2 +
 target/mips/translate.c  | 80 ++--
 target/mips/translate_init.inc.c | 47 +++
 5 files changed, 130 insertions(+), 4 deletions(-)

-- 
2.16.4




[Qemu-devel] [PATCH v4 1/8] target/mips: Define R5900 instructions and CPU preprocessor constants

2018-09-16 Thread Fredrik Noring
The R5900 implements the 64-bit MIPS III instruction set except DMULT,
DMULTU, DDIV, DDIVU, LL, SC, LLD and SCD. The MIPS IV instructions MOVN,
MOVZ and PREF are implemented. It has the R5900 specific three-operand
instructions MADD, MADDU, MULT and MULTU as well as pipeline 1 versions
MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and
MTLO1. A set of 93 128-bit multimedia instructions specific to the
R5900 is also implemented.

The Toshiba TX System RISC TX79 Core Architecture manual describes the
R5900 processor:

http://www.lukasz.dk/files/tx79architecture.pdf

Signed-off-by: Fredrik Noring 
---
 target/mips/mips-defs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index c8e99791ad..76550de2da 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -53,6 +53,7 @@
 #define   ASE_MSA   0x0100
 
 /* Chip specific instructions. */
+#define INSN_R5900   0x1000
 #defineINSN_LOONGSON2E  0x2000
 #defineINSN_LOONGSON2F  0x4000
 #defineINSN_VR54XX 0x8000
@@ -63,6 +64,7 @@
 #defineCPU_MIPS3   (CPU_MIPS2 | ISA_MIPS3)
 #defineCPU_MIPS4   (CPU_MIPS3 | ISA_MIPS4)
 #defineCPU_VR54XX  (CPU_MIPS4 | INSN_VR54XX)
+#define CPU_R5900   (CPU_MIPS3 | INSN_R5900)
 #defineCPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
 #defineCPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
 
-- 
2.16.4




[Qemu-devel] [PATCH v4 7/8] linux-user/mips: Recognise the R5900 CPU model

2018-09-16 Thread Fredrik Noring
Signed-off-by: Fredrik Noring 
---
 linux-user/mips/target_elf.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/mips/target_elf.h b/linux-user/mips/target_elf.h
index fa5d30bf99..a98c9bd6ad 100644
--- a/linux-user/mips/target_elf.h
+++ b/linux-user/mips/target_elf.h
@@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
 if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
 return "mips32r6-generic";
 }
+if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
+return "R5900";
+}
 return "24Kf";
 }
 #endif
-- 
2.16.4




[Qemu-devel] [PATCH v4 2/8] target/mips: Support R5900 specific three-operand MULT and MULTU

2018-09-16 Thread Fredrik Noring
Signed-off-by: Fredrik Noring 
---
 target/mips/translate.c | 53 +
 1 file changed, 53 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ab16cdb911..fb571e278e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -3768,6 +3768,57 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
 tcg_temp_free(t1);
 }
 
+static void gen_mul_r5900(DisasContext *ctx, uint32_t opc,
+  int acc, int rd, int rs, int rt)
+{
+TCGv t0 = tcg_temp_new();
+TCGv t1 = tcg_temp_new();
+
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+
+switch (opc) {
+case OPC_MULT:
+{
+TCGv_i32 t2 = tcg_temp_new_i32();
+TCGv_i32 t3 = tcg_temp_new_i32();
+tcg_gen_trunc_tl_i32(t2, t0);
+tcg_gen_trunc_tl_i32(t3, t1);
+tcg_gen_muls2_i32(t2, t3, t2, t3);
+if (rd)
+tcg_gen_ext_i32_tl(cpu_gpr[rd], t2);
+tcg_gen_ext_i32_tl(cpu_LO[acc], t2);
+tcg_gen_ext_i32_tl(cpu_HI[acc], t3);
+tcg_temp_free_i32(t2);
+tcg_temp_free_i32(t3);
+}
+break;
+case OPC_MULTU:
+{
+TCGv_i32 t2 = tcg_temp_new_i32();
+TCGv_i32 t3 = tcg_temp_new_i32();
+tcg_gen_trunc_tl_i32(t2, t0);
+tcg_gen_trunc_tl_i32(t3, t1);
+tcg_gen_mulu2_i32(t2, t3, t2, t3);
+if (rd)
+tcg_gen_ext_i32_tl(cpu_gpr[rd], t2);
+tcg_gen_ext_i32_tl(cpu_LO[acc], t2);
+tcg_gen_ext_i32_tl(cpu_HI[acc], t3);
+tcg_temp_free_i32(t2);
+tcg_temp_free_i32(t3);
+}
+break;
+default:
+MIPS_INVAL("mul R5900");
+generate_exception_end(ctx, EXCP_RI);
+goto out;
+}
+
+ out:
+tcg_temp_free(t0);
+tcg_temp_free(t1);
+}
+
 static void gen_mul_vr54xx (DisasContext *ctx, uint32_t opc,
 int rd, int rs, int rt)
 {
@@ -22378,6 +22429,8 @@ static void decode_opc_special_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 check_insn(ctx, INSN_VR54XX);
 op1 = MASK_MUL_VR54XX(ctx->opcode);
 gen_mul_vr54xx(ctx, op1, rd, rs, rt);
+} else if (ctx->insn_flags & INSN_R5900) {
+gen_mul_r5900(ctx, op1, 0, rd, rs, rt);
 } else {
 gen_muldiv(ctx, op1, rd & 3, rs, rt);
 }
-- 
2.16.4




[Qemu-devel] [PATCH v4 4/8] target/mips: Add function to signal RI exception unless user only

2018-09-16 Thread Fredrik Noring
The Linux kernel traps and emulates certain instructions. For user only,
QEMU is the kernel, so we emulate those traps by simply emulating the
instructions directly.

Signed-off-by: Fredrik Noring 
---
 target/mips/translate.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index c35be0053b..77d678353e 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -1887,6 +1887,20 @@ static inline void check_insn_opc_removed(DisasContext 
*ctx, int flags)
 }
 }
 
+/*
+ * Unless user only, when the kernel emulates the code, a "reserved
+ * instruction" exception is generated if the CPU has corresponding
+ * flag set which indicates that the instruction has been removed.
+ */
+static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
+{
+#ifndef CONFIG_USER_ONLY
+if (unlikely(ctx->insn_flags & flags)) {
+generate_exception_end(ctx, EXCP_RI);
+}
+#endif
+}
+
 /* This code generates a "reserved instruction" exception if the
CPU does not support 64-bit paired-single (PS) floating point data type */
 static inline void check_ps(DisasContext *ctx)
-- 
2.16.4




[Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

2018-09-16 Thread Fredrik Noring
These MIPS III instructions are unavailable and therefore trapped and
emulated by the Linux kernel.

Signed-off-by: Fredrik Noring 
---
 target/mips/translate.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 77d678353e..327e96307b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -22458,6 +22458,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 case OPC_DMULTU:
 case OPC_DDIV:
 case OPC_DDIVU:
+check_insn_opc_user_only(ctx, INSN_R5900);
 check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 gen_muldiv(ctx, op1, 0, rs, rt);
@@ -24962,6 +24963,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
  break;
 case OPC_LL: /* Load and stores */
 check_insn(ctx, ISA_MIPS2);
+check_insn_opc_user_only(ctx, INSN_R5900);
 /* Fallthrough */
 case OPC_LWL:
 case OPC_LWR:
@@ -24987,6 +24989,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 case OPC_SC:
 check_insn(ctx, ISA_MIPS2);
  check_insn_opc_removed(ctx, ISA_MIPS32R6);
+check_insn_opc_user_only(ctx, INSN_R5900);
  gen_st_cond(ctx, op, rt, rs, imm);
  break;
 case OPC_CACHE:
@@ -25253,9 +25256,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 
 #if defined(TARGET_MIPS64)
 /* MIPS64 opcodes */
+case OPC_LLD:
+check_insn_opc_user_only(ctx, INSN_R5900);
+/* fall through */
 case OPC_LDL:
 case OPC_LDR:
-case OPC_LLD:
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
 /* fall through */
 case OPC_LWU:
@@ -25275,6 +25280,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 break;
 case OPC_SCD:
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
+check_insn_opc_user_only(ctx, INSN_R5900);
 check_insn(ctx, ISA_MIPS3);
 check_mips_64(ctx);
 gen_st_cond(ctx, op, rt, rs, imm);
-- 
2.16.4




[Qemu-devel] [PATCH v4 6/8] target/mips: Define the R5900 CPU

2018-09-16 Thread Fredrik Noring
The primary purpose of this change is to support programs compiled by
GCC for the R5900 target and thereby run R5900 Linux distributions, for
example Gentoo. In particular, this avoids issues with cross compilation.

This change has been tested with Gentoo compiled for R5900, including
native compilation of several packages under QEMU.

Signed-off-by: Fredrik Noring 
---
 target/mips/translate_init.inc.c | 47 
 1 file changed, 47 insertions(+)

diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
index b3320b9dc7..71fd83de06 100644
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -410,6 +410,53 @@ const mips_def_t mips_defs[] =
 .insn_flags = CPU_MIPS32R5 | ASE_MSA,
 .mmu_type = MMU_TYPE_R4000,
 },
+{
+.name = "R5900",
+.CP0_PRid = 0x3800,
+/* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. 
*/
+.CP0_Config0 = (0x3 << 9) | (0x3 << 6) | (0x2 << CP0C0_K0),
+.CP0_Status_rw_bitmask = 0xF4C79C1F,
+#ifdef CONFIG_USER_ONLY
+/*
+ * R5900 hardware traps to the Linux kernel for IEEE 754-1985 and LL/SC
+ * emulation. For user only, QEMU is the kernel, so we emulate the 
traps
+ * by simply emulating the instructions directly.
+ *
+ * Note: Config1 is only used internally, the R5900 has only Config0.
+ */
+.CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),
+.CP0_LLAddr_rw_bitmask = 0x,
+.CP0_LLAddr_shift = 4,
+.CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
+.CP1_fcr31 = 0,
+.CP1_fcr31_rw_bitmask = 0x0183,
+#else
+/*
+ * The R5900 COP1 FPU implements single-precision floating-point
+ * operations but is not entirely IEEE 754-1985 compatible. In
+ * particular,
+ *
+ * - NaN (not a number) and plus/minus infinities are not supported;
+ * - exception mechanisms are not fully supported;
+ * - denormalized numbers are not supported;
+ * - rounding towards nearest and plus/minus infinities are not 
supported;
+ * - computed results usually differs in the least significant bit;
+ * - saturating instructions can differ more than the least 
significant bit.
+ *
+ * Since only rounding towards zero is supported, the two least
+ * significant bits of FCR31 are hardwired to 01.
+ *
+ * FPU emulation is disabled here until it is implemented.
+ *
+ * Note: Config1 is only used internally, the R5900 has only Config0.
+ */
+.CP0_Config1 = (47 << CP0C1_MMU),
+#endif /* !CONFIG_USER_ONLY */
+.SEGBITS = 32,
+.PABITS = 32,
+.insn_flags = CPU_R5900,
+.mmu_type = MMU_TYPE_R4000,
+},
 {
 /* A generic CPU supporting MIPS32 Release 6 ISA.
FIXME: Support IEEE 754-2008 FP.
-- 
2.16.4




[Qemu-devel] [PATCH v4 3/8] target/mips: Support R5900 instructions MOVN, MOVZ and PREF from MIPS IV

2018-09-16 Thread Fredrik Noring
CPU_R5900 is defined as CPU_MIPS3 but it has the MIPS IV instructions
MOVN, MOVZ and PREF as well.

Signed-off-by: Fredrik Noring 
---
 target/mips/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index fb571e278e..c35be0053b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -22402,7 +22402,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
*env, DisasContext *ctx)
 case OPC_MOVN: /* Conditional move */
 case OPC_MOVZ:
 check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
-   INSN_LOONGSON2E | INSN_LOONGSON2F);
+   INSN_LOONGSON2E | INSN_LOONGSON2F | INSN_R5900);
 gen_cond_move(ctx, op1, rd, rs, rt);
 break;
 case OPC_MFHI:  /* Move from HI/LO */
@@ -24986,7 +24986,8 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 break;
 case OPC_PREF:
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
-check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
+check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
+   INSN_R5900);
 /* Treat as NOP. */
 break;
 
-- 
2.16.4




[Qemu-devel] [PATCH v4 8/8] elf: Toshiba/Sony rather than MIPS are the implementors of the R5900

2018-09-16 Thread Fredrik Noring
Sources [1][2] indicate that the Emotion Engine was designed by Toshiba
and licensed to Sony. Others [3][4] claim it was a joint effort. It may
therefore make sense to refer to the CPU as "Toshiba/Sony R5900".

[1] 
http://cs.nyu.edu/courses/spring02/V22.0480-002/projects/aldrich/emotionengine.ppt
[2] http://archive.arstechnica.com/reviews/1q00/playstation2/m-ee-3.html
[3] 
http://docencia.ac.upc.edu/ETSETB/SEGPAR/microprocessors/emotionengine%20(mpr).pdf
[4] http://www.eetimes.com/document.asp?doc_id=1144055

Reported-by: Maciej W. Rozycki 
Signed-off-by: Fredrik Noring 
---
 include/elf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/elf.h b/include/elf.h
index 312f68af81..2510fc7be4 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -76,7 +76,7 @@ typedef int64_t  Elf64_Sxword;
 #define EF_MIPS_MACH_OCTEON2  0x008d  /* Cavium Networks Octeon2 */
 #define EF_MIPS_MACH_OCTEON3  0x008e  /* Cavium Networks Octeon3 */
 #define EF_MIPS_MACH_5400 0x0091  /* NEC VR5400  */
-#define EF_MIPS_MACH_5900 0x0092  /* MIPS R5900  */
+#define EF_MIPS_MACH_5900 0x0092  /* Toshiba/Sony R5900  */
 #define EF_MIPS_MACH_5500 0x0098  /* NEC VR5500  */
 #define EF_MIPS_MACH_9000 0x0099  /* PMC-Sierra's RM9000 */
 #define EF_MIPS_MACH_LS2E 0x00a0  /* ST Microelectronics Loongson 2E */
-- 
2.16.4




Re: [Qemu-devel] [PATCH v3] target/mips: Support R5900 GCC programs in user mode

2018-09-16 Thread Maciej W. Rozycki
Hi Fredrik,

> Many thanks for your review, Maciej,

 You are welcome!

> >  Eventually you'll have to remove all these instructions (plus LL and SC) 
> > from the system emulation mode.  In fact I think it would make sense to do 
> > that right away, because I believe it will be a reasonably simple update. 
> > However if it turns out to be a can of worms after all, then I think we 
> > can defer it, because we do not have a bare-metal environment ready for 
> > this CPU anyway (or do we?).
> 
> Indeed, very simple. I added a new check function check_insn_opc_user_only,
> similar to check_insn_opc_removed, conditional on USER_ONLY and so a simple
> 
>   check_insn_opc_user_only(ctx, INSN_R5900);
> 
> has to be added for these OPC cases. To test this I tried to build GCC with
> the target mips64r5900el but it unfortunately didn't work as GCC failed to
> compile itself:
> 
>   ../sysdeps/mips/mips64/mul_1.S:49: Error: opcode not supported on
>   this processor: r5900 (mips3) `dmultu $8,$7'

 That file is a part of GMP rather that GCC I believe.  A copy is used 
by glibc too.

 I think you'll have to arrange for GMP and glibc builds to avoid this 
file for R5900 targets, either by supplying a handcoded R5900 assembly 
alternative that avoids the missing instruction (possibly by using a 
sequence of MULTU operations), or by using the generic C-language 
version from mul_1.c.

 I wouldn't recommend placing an architecture override within the 
generic 64-bit MIPS assembly so as to force it to build, as any kernel 
DMULTU emulation is bound to be slower, because it'll have to do all 
that we need to do here anyway, and then it'll add the overhead of 
calling an exception handler.

 I suspect there will be more such cases to handle in GMP and glibc.  
As GMP holds the master copies of code you'll need to submit any updates 
to that project first, and once accepted they can be carried over to 
glibc.

 As I suggested before you may wish to focus on making o32 work first 
rather than keeping being distracted by 64-bit issues, so that we have 
at least some functional code upstream before diving into enhancements 
and corner cases.

 HTH,

  Maciej



Re: [Qemu-devel] [PATCH v2 12/17] mirror: Fix potential use-after-free in active commit

2018-09-16 Thread Max Reitz
On 14.09.18 18:25, Kevin Wolf wrote:
> Am 13.09.2018 um 22:55 hat Max Reitz geschrieben:
>> On 13.09.18 14:52, Kevin Wolf wrote:
>>> When starting an active commit job, other callbacks can run before
>>> mirror_start_job() calls bdrv_ref() where needed and cause the nodes to
>>> go away. Add another pair of bdrv_ref/unref() around it to protect
>>> against this case.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  block/mirror.c | 11 +++
>>>  1 file changed, 11 insertions(+)
>>
>> Reviewed-by: Max Reitz 
>>
>> But...  How?
> 
> Tried to reproduce the other bug Paolo was concerned about (good there
> is something like 'git reflog'!) and dug up this one instead.
> 
> So the scenario seems to be test_stream_commit_1 in qemu-iotests 030.
> 
> The backtrace when the BDS is deleted is the following:
> 
> (rr) bt
> #0  0x7faeaf6145ec in __memset_sse2_unaligned_erms () at /lib64/libc.so.6
> #1  0x7faeaf60414e in _int_free () at /lib64/libc.so.6
> #2  0x7faeaf6093be in free () at /lib64/libc.so.6
> #3  0x7faecaea6b4e in g_free () at /lib64/libglib-2.0.so.0
> #4  0x55c0c94f9d6c in bdrv_delete (bs=0x55c0cbe69240) at block.c:3590
> #5  0x55c0c94fbe82 in bdrv_unref (bs=0x55c0cbe69240) at block.c:4638
> #6  0x55c0c94f6761 in bdrv_root_unref_child (child=0x55c0cbe57af0) at 
> block.c:2188
> #7  0x55c0c94fe239 in block_job_remove_all_bdrv (job=0x55c0ccf9e220) at 
> blockjob.c:200
> #8  0x55c0c94fdf1f in block_job_free (job=0x55c0ccf9e220) at blockjob.c:94
> #9  0x55c0c94ffe0d in job_unref (job=0x55c0ccf9e220) at job.c:368
> #10 0x55c0c95007cd in job_do_dismiss (job=0x55c0ccf9e220) at job.c:641
> #11 0x55c0c95008d9 in job_conclude (job=0x55c0ccf9e220) at job.c:667
> #12 0x55c0c9500b66 in job_finalize_single (job=0x55c0ccf9e220) at 
> job.c:735
> #13 0x55c0c94ff4fa in job_txn_apply (txn=0x55c0cbe19eb0, 
> fn=0x55c0c9500a62 , lock=true) at job.c:151
> #14 0x55c0c9500e92 in job_do_finalize (job=0x55c0ccf9e220) at job.c:822
> #15 0x55c0c9501040 in job_completed_txn_success (job=0x55c0ccf9e220) at 
> job.c:872
> #16 0x55c0c950115c in job_completed (job=0x55c0ccf9e220, ret=0, 
> error=0x0) at job.c:892
> #17 0x55c0c92b572c in stream_complete (job=0x55c0ccf9e220, 
> opaque=0x55c0cc050bc0) at block/stream.c:96

But isn't this just deletion of node1, i.e. the intermediate node of the
stream job?

The commit job happens above that (node3 and upwards), so all its BDSs
shouldn't be affected.  So even with the bdrv_ref()s introduced in this
patch, I'd still expect node1 to be deleted exactly here.

Max

> #18 0x55c0c950142b in job_defer_to_main_loop_bh (opaque=0x55c0cbe38a50) 
> at job.c:981
> #19 0x55c0c96171fc in aio_bh_call (bh=0x55c0cbe19f20) at util/async.c:90
> #20 0x55c0c9617294 in aio_bh_poll (ctx=0x55c0cbd7b8d0) at util/async.c:118
> #21 0x55c0c961c150 in aio_poll (ctx=0x55c0cbd7b8d0, blocking=true) at 
> util/aio-posix.c:690
> #22 0x55c0c957246c in bdrv_flush (bs=0x55c0cbe4b250) at block/io.c:2693
> #23 0x55c0c94f8e46 in bdrv_reopen_prepare (reopen_state=0x55c0cbef4d68, 
> queue=0x55c0cbeab9f0, errp=0x7ffd65617050) at block.c:3206
> #24 0x55c0c94f883a in bdrv_reopen_multiple (ctx=0x55c0cbd7b8d0, 
> bs_queue=0x55c0cbeab9f0, errp=0x7ffd656170c8) at block.c:3032
> #25 0x55c0c94f8a00 in bdrv_reopen (bs=0x55c0cbe2c250, bdrv_flags=8194, 
> errp=0x7ffd65617220) at block.c:3075
> #26 0x55c0c956a23f in commit_active_start (job_id=0x0, bs=0x55c0cbd905b0, 
> base=0x55c0cbe2c250, creation_flags=0, speed=0, 
> on_error=BLOCKDEV_ON_ERROR_REPORT, filter_node_name=0x0, cb=0x0, opaque=0x0, 
> auto_complete=false, errp=0x7ffd65617220) at block/mirror.c:1687
> #27 0x55c0c927437c in qmp_block_commit (has_job_id=false, job_id=0x0, 
> device=0x55c0cbef4d20 "drive0", has_base_node=false, base_node=0x0, 
> has_base=true, base=0x55c0cbe38a00 
> "/home/kwolf/source/qemu/tests/qemu-iotests/scratch/img-3.img", 
> has_top_node=false, top_node=0x0, has_top=false, top=0x0, 
> has_backing_file=false, backing_file=0x0, has_speed=false, speed=0, 
> has_filter_node_name=false, filter_node_name=0x0, errp=0x7ffd656172d8) at 
> blockdev.c:3325
> #28 0x55c0c928bb08 in qmp_marshal_block_commit (args=, 
> ret=, errp=0x7ffd656173b8) at 
> qapi/qapi-commands-block-core.c:409
> #29 0x55c0c960beef in do_qmp_dispatch (errp=0x7ffd656173b0, 
> allow_oob=false, request=, cmds=0x55c0c9f56f80 ) 
> at qapi/qmp-dispatch.c:129
> #30 0x55c0c960beef in qmp_dispatch (cmds=0x55c0c9f56f80 , 
> request=, allow_oob=) at qapi/qmp-dispatch.c:171
> #31 0x55c0c9111623 in monitor_qmp_dispatch (mon=0x55c0cbdbb930, 
> req=0x55c0ccf90e00, id=0x0) at /home/kwolf/source/qemu/monitor.c:4168
> #32 0x55c0c911191f in monitor_qmp_bh_dispatcher (data=0x0) at 
> /home/kwolf/source/qemu/monitor.c:4237
> #33 0x55c0c96171fc in aio_bh_call (bh=0x55c0cbccc840) at util/async.c:90
> #34 0x55c0c9617294 in aio_bh_poll (ctx=0x55c0cbccc550) at util/async.c:118
> #35 0

Re: [Qemu-devel] [PATCH v4 7/8] linux-user/mips: Recognise the R5900 CPU model

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/15/18 11:08 AM, Fredrik Noring wrote:
> Signed-off-by: Fredrik Noring 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  linux-user/mips/target_elf.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/linux-user/mips/target_elf.h b/linux-user/mips/target_elf.h
> index fa5d30bf99..a98c9bd6ad 100644
> --- a/linux-user/mips/target_elf.h
> +++ b/linux-user/mips/target_elf.h
> @@ -12,6 +12,9 @@ static inline const char *cpu_get_model(uint32_t eflags)
>  if ((eflags & EF_MIPS_ARCH) == EF_MIPS_ARCH_32R6) {
>  return "mips32r6-generic";
>  }
> +if ((eflags & EF_MIPS_MACH) == EF_MIPS_MACH_5900) {
> +return "R5900";
> +}
>  return "24Kf";
>  }
>  #endif
> 



Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/16/18 5:13 PM, Fredrik Noring wrote:
> These MIPS III instructions are unavailable and therefore trapped and
> emulated by the Linux kernel.
> 
> Signed-off-by: Fredrik Noring 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/translate.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 77d678353e..327e96307b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -22458,6 +22458,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>  case OPC_DMULTU:
>  case OPC_DDIV:
>  case OPC_DDIVU:
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  check_insn(ctx, ISA_MIPS3);
>  check_mips_64(ctx);
>  gen_muldiv(ctx, op1, 0, rs, rt);
> @@ -24962,6 +24963,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>   break;
>  case OPC_LL: /* Load and stores */
>  check_insn(ctx, ISA_MIPS2);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  /* Fallthrough */
>  case OPC_LWL:
>  case OPC_LWR:
> @@ -24987,6 +24989,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  case OPC_SC:
>  check_insn(ctx, ISA_MIPS2);
>   check_insn_opc_removed(ctx, ISA_MIPS32R6);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>   gen_st_cond(ctx, op, rt, rs, imm);
>   break;
>  case OPC_CACHE:
> @@ -25253,9 +25256,11 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  
>  #if defined(TARGET_MIPS64)
>  /* MIPS64 opcodes */
> +case OPC_LLD:
> +check_insn_opc_user_only(ctx, INSN_R5900);
> +/* fall through */
>  case OPC_LDL:
>  case OPC_LDR:
> -case OPC_LLD:
>  check_insn_opc_removed(ctx, ISA_MIPS32R6);
>  /* fall through */
>  case OPC_LWU:
> @@ -25275,6 +25280,7 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  break;
>  case OPC_SCD:
>  check_insn_opc_removed(ctx, ISA_MIPS32R6);
> +check_insn_opc_user_only(ctx, INSN_R5900);
>  check_insn(ctx, ISA_MIPS3);
>  check_mips_64(ctx);
>  gen_st_cond(ctx, op, rt, rs, imm);
> 



Re: [Qemu-devel] [PATCH v4 8/8] elf: Toshiba/Sony rather than MIPS are the implementors of the R5900

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/15/18 12:28 PM, Fredrik Noring wrote:
> Sources [1][2] indicate that the Emotion Engine was designed by Toshiba
> and licensed to Sony. Others [3][4] claim it was a joint effort. It may
> therefore make sense to refer to the CPU as "Toshiba/Sony R5900".

This looks fair.

BTW Maciej do you remember what TMPR stands for?
I guess remember (T)oshiba (M)icro(P)rocessor (R)ISC from the time MIPS
was the RISC leader (cpu core is MIPS).
Then they started the TMPA SoC series with a (A)RM9 core.
Recently they launched the TMPM with a ARM Cortex-(M) core.

> [1] 
> http://cs.nyu.edu/courses/spring02/V22.0480-002/projects/aldrich/emotionengine.ppt
> [2] http://archive.arstechnica.com/reviews/1q00/playstation2/m-ee-3.html
> [3] 
> http://docencia.ac.upc.edu/ETSETB/SEGPAR/microprocessors/emotionengine%20(mpr).pdf
> [4] http://www.eetimes.com/document.asp?doc_id=1144055
> 
> Reported-by: Maciej W. Rozycki 
> Signed-off-by: Fredrik Noring 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  include/elf.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/elf.h b/include/elf.h
> index 312f68af81..2510fc7be4 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -76,7 +76,7 @@ typedef int64_t  Elf64_Sxword;
>  #define EF_MIPS_MACH_OCTEON2  0x008d  /* Cavium Networks Octeon2 
> */
>  #define EF_MIPS_MACH_OCTEON3  0x008e  /* Cavium Networks Octeon3 
> */
>  #define EF_MIPS_MACH_5400 0x0091  /* NEC VR5400  
> */
> -#define EF_MIPS_MACH_5900 0x0092  /* MIPS R5900  
> */
> +#define EF_MIPS_MACH_5900 0x0092  /* Toshiba/Sony R5900  
> */
>  #define EF_MIPS_MACH_5500 0x0098  /* NEC VR5500  
> */
>  #define EF_MIPS_MACH_9000 0x0099  /* PMC-Sierra's RM9000 
> */
>  #define EF_MIPS_MACH_LS2E 0x00a0  /* ST Microelectronics Loongson 2E 
> */
> 



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v4 1/8] target/mips: Define R5900 instructions and CPU preprocessor constants

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/7/18 7:43 PM, Fredrik Noring wrote:
> The R5900 implements the 64-bit MIPS III instruction set except DMULT,
> DMULTU, DDIV, DDIVU, LL, SC, LLD and SCD. The MIPS IV instructions MOVN,
> MOVZ and PREF are implemented. It has the R5900 specific three-operand
> instructions MADD, MADDU, MULT and MULTU as well as pipeline 1 versions
> MULT1, MULTU1, DIV1, DIVU1, MADD1, MADDU1, MFHI1, MFLO1, MTHI1 and
> MTLO1. A set of 93 128-bit multimedia instructions specific to the
> R5900 is also implemented.
> 
> The Toshiba TX System RISC TX79 Core Architecture manual describes the
> R5900 processor:
> 
> http://www.lukasz.dk/files/tx79architecture.pdf
> 
> Signed-off-by: Fredrik Noring 
> ---
>  target/mips/mips-defs.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
> index c8e99791ad..76550de2da 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -53,6 +53,7 @@
>  #define   ASE_MSA   0x0100
>  
>  /* Chip specific instructions. */
> +#define INSN_R5900   0x1000
>  #define  INSN_LOONGSON2E  0x2000
>  #define  INSN_LOONGSON2F  0x4000
>  #define  INSN_VR54XX 0x8000
> @@ -63,6 +64,7 @@
>  #define  CPU_MIPS3   (CPU_MIPS2 | ISA_MIPS3)
>  #define  CPU_MIPS4   (CPU_MIPS3 | ISA_MIPS4)

The following definitions are used only once for the mips_def_t member.
I plan to remove them later, anyway until then:

Reviewed-by: Philippe Mathieu-Daudé 

>  #define  CPU_VR54XX  (CPU_MIPS4 | INSN_VR54XX)
> +#define CPU_R5900   (CPU_MIPS3 | INSN_R5900)
>  #define  CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
>  #define  CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F)
>  
> 



Re: [Qemu-devel] [PATCH v4 3/8] target/mips: Support R5900 instructions MOVN, MOVZ and PREF from MIPS IV

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/15/18 10:43 AM, Fredrik Noring wrote:
> CPU_R5900 is defined as CPU_MIPS3 but it has the MIPS IV instructions
> MOVN, MOVZ and PREF as well.
> 
> Signed-off-by: Fredrik Noring 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/translate.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index fb571e278e..c35be0053b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -22402,7 +22402,7 @@ static void decode_opc_special_legacy(CPUMIPSState 
> *env, DisasContext *ctx)
>  case OPC_MOVN: /* Conditional move */
>  case OPC_MOVZ:
>  check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
> -   INSN_LOONGSON2E | INSN_LOONGSON2F);
> +   INSN_LOONGSON2E | INSN_LOONGSON2F | INSN_R5900);
>  gen_cond_move(ctx, op1, rd, rs, rt);
>  break;
>  case OPC_MFHI:  /* Move from HI/LO */
> @@ -24986,7 +24986,8 @@ static void decode_opc(CPUMIPSState *env, 
> DisasContext *ctx)
>  break;
>  case OPC_PREF:
>  check_insn_opc_removed(ctx, ISA_MIPS32R6);
> -check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
> +check_insn(ctx, ISA_MIPS4 | ISA_MIPS32 |
> +   INSN_R5900);
>  /* Treat as NOP. */
>  break;
>  
> 



Re: [Qemu-devel] [PATCH v4 4/8] target/mips: Add function to signal RI exception unless user only

2018-09-16 Thread Philippe Mathieu-Daudé
On 9/16/18 5:04 PM, Fredrik Noring wrote:
> The Linux kernel traps and emulates certain instructions. For user only,
> QEMU is the kernel, so we emulate those traps by simply emulating the
> instructions directly.
> 
> Signed-off-by: Fredrik Noring 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/mips/translate.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c35be0053b..77d678353e 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1887,6 +1887,20 @@ static inline void check_insn_opc_removed(DisasContext 
> *ctx, int flags)
>  }
>  }
>  
> +/*
> + * Unless user only, when the kernel emulates the code, a "reserved
> + * instruction" exception is generated if the CPU has corresponding
> + * flag set which indicates that the instruction has been removed.
> + */
> +static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
> +{
> +#ifndef CONFIG_USER_ONLY
> +if (unlikely(ctx->insn_flags & flags)) {
> +generate_exception_end(ctx, EXCP_RI);
> +}
> +#endif
> +}
> +
>  /* This code generates a "reserved instruction" exception if the
> CPU does not support 64-bit paired-single (PS) floating point data type */
>  static inline void check_ps(DisasContext *ctx)
> 



Re: [Qemu-devel] [PATCH] clean up callback when del virtqueue

2018-09-16 Thread Jason Wang




On 2018年09月14日 21:14, liujunjie (A) wrote:



-Original Message-
From: Jason Wang [mailto:jasow...@redhat.com]
Sent: Friday, September 14, 2018 8:45 PM
To: liujunjie (A) ; m...@redhat.com
Cc: Huangweidong (C) ; wangxin (U)
; qemu-devel@nongnu.org; Gonglei (Arei)
; Zhoujian (jay) 
Subject: Re: [Qemu-devel] [PATCH] clean up callback when del virtqueue



On 2018年09月08日 21:04, liujunjie wrote:

Before, we did not clear callback like handle_output when delete the
virtqueue which may result be segmentfault.
The scene is as follows:
1. Start a vm with multiqueue vhost-net, 2. then we write
VIRTIO_PCI_GUEST_FEATURES in PCI configuration to triger multiqueue
disable in this vm which will delete the virtqueue.
In this step, the tx_bh is deleted but the callback
virtio_net_handle_tx_bh still exist.
3. Finally, we write VIRTIO_PCI_QUEUE_NOTIFY in PCI configuration to
notify the deleted virtqueue. In this way, virtio_net_handle_tx_bh
will be called and qemu will be crashed.

Good catch.


Although the way described above is uncommon, we had better reinforce it.

Signed-off-by: liujunjie 
---
   hw/net/virtio-net.c | 4 +++-
   hw/virtio/virtio.c  | 3 +++
   2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index
f154756..9bb20e3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1467,7 +1467,9 @@ static void virtio_net_handle_tx_bh(VirtIODevice

*vdev, VirtQueue *vq)

   return;
   }
   virtio_queue_set_notification(vq, 0);
-qemu_bh_schedule(q->tx_bh);
+if (q->tx_bh) {
+qemu_bh_schedule(q->tx_bh);
+}

I believe this is not necessary since handle_output is NULL now?

Yes, it is not necessary.


   }

   static void virtio_net_tx_timer(void *opaque) diff --git
a/hw/virtio/virtio.c b/hw/virtio/virtio.c index d4e4d98..7577518
100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1604,6 +1604,9 @@ void virtio_del_queue(VirtIODevice *vdev, int n)

   vdev->vq[n].vring.num = 0;
   vdev->vq[n].vring.num_default = 0;
+vdev->vq[n].vring.align = 0;

Is this related or fix for another bug?

It's not related for another bug. So it is also unnecessary to add this, too.
The reason why add it is only trying to correspond to the function 
 in the same file.


Ok, please send V2 and cc qemu stable.

Thanks




Thanks


+vdev->vq[n].handle_output = NULL;
+vdev->vq[n].handle_aio_output = NULL;
   }

   static void virtio_set_isr(VirtIODevice *vdev, int value)





[Qemu-devel] [PATCH] qom: fix comments for object_property_set_qobject function

2018-09-16 Thread Li Qiang
Also make the definition and declare of this function's argument name
the same.

Signed-off-by: Li Qiang 
---
 include/qom/qom-qobject.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h
index 77cd717e3f..11803c6b57 100644
--- a/include/qom/qom-qobject.h
+++ b/include/qom/qom-qobject.h
@@ -30,13 +30,13 @@ struct QObject *object_property_get_qobject(Object *obj, 
const char *name,
 /**
  * object_property_set_qobject:
  * @obj: the object
- * @ret: The value that will be written to the property.
+ * @value: The value that will be written to the property.
  * @name: the name of the property
  * @errp: returns an error if this function fails
  *
  * Writes a property to a object.
  */
-void object_property_set_qobject(Object *obj, struct QObject *qobj,
+void object_property_set_qobject(Object *obj, struct QObject *value,
  const char *name, struct Error **errp);
 
 #endif
-- 
2.11.0




Re: [Qemu-devel] Can I only commit from active image to corresponding range of its backing file by qemu cmd?

2018-09-16 Thread lampahome
>
>
> *(That's what I said range is different)*
>> Ex: 1st chunk of device will save into image.000
>> 2nd chunk of device will save into image.001
>> Nth chunk of device will save into image.(N-1)
>> ...etc
>>
>> I can see all block device data when I mount image.(N-1) by qemu-nbd cuz
>> the chunk doesn't overlap and all chunks connect by backing chain.
>>
>
> How exactly did you create those images?  I'm trying to verify the steps
> you used to split the image. I know the concept of the split, but without
> seeing actual commands used, I don't know that you actually accomplished
> the split in the manner desired.  (It's okay if a reproduction uses smaller
> scales for speed, such as splitting a 32M image across 1M qcow2 files - the
> point remains that seeing the actual steps used may offer additional
> insights into your usage scenario).
>

I create those images by my own python scriopt. I research the behaviors
when read/write/discard...etc from qemu github.
If every image size limit I set is 1TB, script will create next image
automatically when refcount of the whole image is all up to 1 and then
redirect read/write to the new image.


> Or are you trying to ask if it is possible to create such a fragmented
> design with current tools?  (The answer that we've given you is that no, it
> is not easy to do, because no one has needed it so far).  There's no way to
> tell a running qemu that writes to offsets 0-1M go into one file, while
> writes to offsets 1M to 2M go into another - ALL writes go into the
> currently active layer, regardless of the offset represented by the write.
>
Got it, thx.


>
>> So now I have two *version of block device(like concept of snapshot)*:
>> One is image.000 to image.(N-1). I can access the data before modify by
>> mount image.(N-1) through qemu-nbd
>> The other one is image.000 to image.N.  I can access the data after modify
>> by mount image.N through qemu-nbd(cuz the visible 1st chunk are in the
>> image.N)
>>
>> Consider about the situation:
>> 000   A - - - - - - - -  <---  store the 1st chunk of block device
>> 001   - B - - - - - - -
>> 002   - - C - - - - - - (1st state of block device)
>> 003   A' - - - - - - - - <--- store the 1st chunk of block device, but
>> data is different
>> 004   - - - D - - - - - (2nd state of block device)
>> 005   - - - - E - - - -  (3rd state of block device)
>>
>> The original problem is If I want to remove the 2nd state(003 and 004) but
>> I need to keep the data of 003 and 004.
>> If I just commit 003, the A' of 003 must be committed into 002 cuz 002 is
>> the backing file of 003.
>> I try to figure out some way to let it only commit from 003 into 000.
>>
>>
> I'm not quite following your diagram. My naive read (probably wrong) is
> that you are trying to present a 9M image (scaled M to G or T as
> appropriate) to the guest, as represented by the 9 characters, but that the
> initial image only populated 3M of the 9 with guest-visible contents
> represented by ABC--.  So you want to split that into files 000
> containing offsets 0-1M (A), 001 containing offsets 1M-2M
> (-B---), and 002 containing offsets 2M-3M (--C--).  Then you want
> to run the guest, which does some modifications in offsets 0-1M (I'll write
> it as "a" instead of "A'", you could also have chosen a different letter
> except that your example already uses "D" elsewhere), so the guest now sees
> (aBC--), and you want to store that incremental backup in file 003,
> containing just (a).  But that's where I got confused - my original
> assumption was that 003 represented offsets 3M-4M (---X-), but you are
> now showing it as representing offsets 0-1M.  It's also not clear which
> files in your list have which other files as backing files.
>
> You can view it as snapshots. From 000~002 are the data of 1st snapshot.
and 003~004 are the 2nd snapshot. The difference of two snapshot is the 1st
chunk(A is in the 1st snapshot and A' is in the 2nd snapshot).
So that's why the A' writes in 1st chunk of 003(because 1st of 003 and 000
are the different data).

Incremental backup by programming way is just a thought because I try to
conquer the file size limit
I indeed try rebase and commit command to do this, but still can't do this.
Now maybe the case is not normal by everyone. I try to figure out another
way or use case to try incremental backup by my own script.


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


Re: [Qemu-devel] [PATCH v3 0/2] 40p: fix PCI interrupt routing

2018-09-16 Thread David Gibson
On Mon, Sep 10, 2018 at 09:46:29PM +0100, Mark Cave-Ayland wrote:
> According to the PReP specification section 6.1.6 "System Interrupt
> Assignments", all PCI interrupts are routed via IRQ 15.
> 
> In the case of the 40p machine this isn't quite true in that it has a routing
> quirk: the LSI SCSI device is always routed to IRQ 13. At least Linux and
> NetBSD compare the model name presented by the firmware to "IBM PPS Model
> 6015", and if it matches will active this quirk.
> 
> In order for guest OSs to make use of the fixed IRQ routing, the model name
> in the residual data must be changed in OpenBIOS using the diff below:
> 
> diff --git a/arch/ppc/qemu/context.c b/arch/ppc/qemu/context.c
> index 06e0122..5815895 100644
> --- a/arch/ppc/qemu/context.c
> +++ b/arch/ppc/qemu/context.c
> @@ -111,7 +111,7 @@ static void *
>  residual_build(uint32_t memsize, uint32_t load_base, uint32_t load_size)
>  {
>  residual_t *res;
> -const unsigned char model[] = "Qemu\0PPC\0";
> +const unsigned char model[] = "IBM PPS Model 6015\0";
>  int i;
>  
>  res = malloc(sizeof(residual_t));
> 
> With the above OpenBIOS patch applied as well as this patchset, it is now
> possible to boot the sandalfoot zImage all the way through to a working
> userspace when using OpenBIOS.
> 
> (Note: this patchset requires the changes in my previous patchset "scsi:
> replace lsi53c895a_create() and lsi53c810_create() functions)
> 
> Signed-off-by: Mark Cave-Ayland 
> Based-on: <20180907125653.5010-1-mark.cave-ayl...@ilande.co.uk>

Mark,

I think we have all the necessary acks to go ahead with this.
However, I'm afraid I've lost track of the various prereq patches that
were necessary here.  Can you resend with all the necessary pieces
rebased against ppc-for-3.1 and the appropriate acked-bys included?

> 
> v3:
> - Add external IRQ to LSI SCSI device instead of hacking the PCI interrupt
>   routing as suggested by David
> - Rebase onto the patches from v2 already applied to ppc-for-3.1
> 
> v2:
> - Add OR gate as recommended by Zoltan and implement routing quirk by hacking
>   the raven PCI interrupt routing
> 
> 
> Mark Cave-Ayland (2):
>   lsi53c895a: add optional external IRQ via qdev
>   40p: add fixed IRQ routing for LSI SCSI device
> 
>  hw/ppc/prep.c| 11 ++-
>  hw/scsi/lsi53c895a.c | 16 ++--
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 0/2] migration cleanup fixes

2018-09-16 Thread Peter Xu
On Fri, Sep 14, 2018 at 06:04:28PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> This is a pair of related migration fixes for cases where the loadvm
> state doesn't get cleaned up.
> 
> In the first case this is because the code hasn't cleared the postcopy
> flag, and so a following loadvm gets very confused (leading to it
> not cleaning up the loadvm state or syncing the CPUs leading to no
> interrupts and a very confused guest).
> 
> In the second case I just noticed a few return cases where it didn't get
> cleaned up in error paths.

Reviewed-by: Peter Xu 

Regards,

-- 
Peter Xu



Re: [Qemu-devel] [PATCH v3 0/2] 40p: fix PCI interrupt routing

2018-09-16 Thread Mark Cave-Ayland
On 17/09/18 04:54, David Gibson wrote:

> On Mon, Sep 10, 2018 at 09:46:29PM +0100, Mark Cave-Ayland wrote:
>> According to the PReP specification section 6.1.6 "System Interrupt
>> Assignments", all PCI interrupts are routed via IRQ 15.
>>
>> In the case of the 40p machine this isn't quite true in that it has a routing
>> quirk: the LSI SCSI device is always routed to IRQ 13. At least Linux and
>> NetBSD compare the model name presented by the firmware to "IBM PPS Model
>> 6015", and if it matches will active this quirk.
>>
>> In order for guest OSs to make use of the fixed IRQ routing, the model name
>> in the residual data must be changed in OpenBIOS using the diff below:
>>
>> diff --git a/arch/ppc/qemu/context.c b/arch/ppc/qemu/context.c
>> index 06e0122..5815895 100644
>> --- a/arch/ppc/qemu/context.c
>> +++ b/arch/ppc/qemu/context.c
>> @@ -111,7 +111,7 @@ static void *
>>  residual_build(uint32_t memsize, uint32_t load_base, uint32_t load_size)
>>  {
>>  residual_t *res;
>> -const unsigned char model[] = "Qemu\0PPC\0";
>> +const unsigned char model[] = "IBM PPS Model 6015\0";
>>  int i;
>>  
>>  res = malloc(sizeof(residual_t));
>>
>> With the above OpenBIOS patch applied as well as this patchset, it is now
>> possible to boot the sandalfoot zImage all the way through to a working
>> userspace when using OpenBIOS.
>>
>> (Note: this patchset requires the changes in my previous patchset "scsi:
>> replace lsi53c895a_create() and lsi53c810_create() functions)
>>
>> Signed-off-by: Mark Cave-Ayland 
>> Based-on: <20180907125653.5010-1-mark.cave-ayl...@ilande.co.uk>
> 
> Mark,
> 
> I think we have all the necessary acks to go ahead with this.
> However, I'm afraid I've lost track of the various prereq patches that
> were necessary here.  Can you resend with all the necessary pieces
> rebased against ppc-for-3.1 and the appropriate acked-bys included?

Sure, no problem. I'll resend it this evening as a new 40p PCI routing
"roll-up" patchset.


ATB,

Mark.



Re: [Qemu-devel] [PATCH v2 3/8] x86_iommu/amd: remove V=1 check from amdvi_validate_dte()

2018-09-16 Thread Peter Xu
On Fri, Sep 14, 2018 at 01:26:58PM -0500, Brijesh Singh wrote:
> Currently, the amdvi_validate_dte() assumes that a valid DTE will
> always have V=1. This is not true. The V=1 means that bit[127:1] are
> valid. A valid DTE can have IV=1 and V=0 (i.e pt=off, intremap=on).

"pt" might be a bit confusing here.  Now "intel-iommu" device has the
"pt" parameter to specify IOMMU DMAR passthrough support.  Also the
corresponding guest kernel parameter "iommu_pt".  So I would suggest
to use "page translation" (is this really the term that AMD spec is
used after all?) or directly DMAR (DMA remapping).

> 
> Remove the V=1 check from amdvi_validate_dte(), make the caller
> responsible to check for V or IV bits.
> 
> Signed-off-by: Brijesh Singh 
> Cc: "Michael S. Tsirkin" 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcel Apfelbaum 
> Cc: Tom Lendacky 
> Cc: Suravee Suthikulpanit 
> ---
>  hw/i386/amd_iommu.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 1fd669f..225825e 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -807,7 +807,7 @@ static inline uint64_t amdvi_get_perms(uint64_t entry)
> AMDVI_DEV_PERM_SHIFT;
>  }
>  
> -/* a valid entry should have V = 1 and reserved bits honoured */
> +/* validate that reserved bits are honoured */
>  static bool amdvi_validate_dte(AMDVIState *s, uint16_t devid,
> uint64_t *dte)
>  {
> @@ -820,7 +820,7 @@ static bool amdvi_validate_dte(AMDVIState *s, uint16_t 
> devid,
>  return false;
>  }
>  
> -return dte[0] & AMDVI_DEV_VALID;
> +return true;
>  }
>  
>  /* get a device table entry given the devid */
> @@ -967,7 +967,8 @@ static void amdvi_do_translate(AMDVIAddressSpace *as, 
> hwaddr addr,
>  }
>  
>  /* devices with V = 0 are not translated */
> -if (!amdvi_get_dte(s, devid, entry)) {
> +if (!amdvi_get_dte(s, devid, entry) &&
> +!(entry[0] & AMDVI_DEV_VALID)) {

Here I'm not sure whether you're considering endianess.  I think
amdvi_get_dte() tried to fix the endianess somehow but I'm not sure
it's complete (so entry[0] is special here...):

static bool amdvi_get_dte(AMDVIState *s, int devid, uint64_t *entry)
{
uint32_t offset = devid * AMDVI_DEVTAB_ENTRY_SIZE;

if (dma_memory_read(&address_space_memory, s->devtab + offset, entry,
AMDVI_DEVTAB_ENTRY_SIZE)) {
trace_amdvi_dte_get_fail(s->devtab, offset);
/* log error accessing dte */
amdvi_log_devtab_error(s, devid, s->devtab + offset, 0);
return false;
}

*entry = le64_to_cpu(*entry);  <--- [1]
if (!amdvi_validate_dte(s, devid, entry)) {
trace_amdvi_invalid_dte(entry[0]);
return false;
}

return true;
}

At [1] only one 64bits entry is swapped correctly to cpu endianess,
IMHO the rest of the three uint64_t is still using LE.

I'm not really sure whether there would be anyone that wants to run
the AMD IOMMU on big endian hosts, but I just want to know the goal of
this series - do you want to support this scenario?  If so, you might
need to fixup the places too AFAIU.

>  goto out;
>  }
>  
> -- 
> 2.7.4
> 
> 

Regards,

-- 
Peter Xu



Re: [Qemu-devel] [PATCH v2] usb: assign unique serial numbers to hid devices

2018-09-16 Thread Gerd Hoffmann
> > And even if it turns out autosuspend is still an issue:  I think
> > meanwhile we can really stop worrying about guests running in old qemu
> > versions with broken usb suspend (fixed in 0.13 !).  If needed we can
> > enable autosuspend unconditionally in guests.
> 
> OK, so what about the other question - which serial number do we end up
> with after this patch; do we get the 89126/28754/68284 or do we get the
> path based one from your change that calls usb_desc_create_serial?

The path based one. and the numbers are the first part, i.e. this:

root@fedora ~# lsusb -vs1:2
[ ... ]
  iManufacturer   1 QEMU
  iProduct3 QEMU USB Tablet
  iSerial 6 28754-:00:02.1:00.0-1
  bNumConfigurations  1
[ ... ]

cheers,
  Gerd