[Qemu-devel] Guest Reboot After Plug HDMI

[Lying]

Hello, Everybody. I encounter a problem when i using my guest.

I booting my guest without HDMI primarily, Then i add it, but my guest is be 
rebooted.


To know what cause it, i do it again, especially i check my guest is running 
normal or not with "ping" before plugging, that's well.


What i was suffer? can you help me solve it?


Following message is detailed!!!


Guest: window 7 profession


Linux: Ubuntu 1604


Qemu:1 v2.11.0-rc0 


Libvirt: 3.9


method of  booting: virsh start myGuest1


method of plug HDMI: virsh qemu-monitor-command myGuest1 --hmp 'device_add 
driver=vfio-pci,host=01:00.0,xvga=on,multifunction=yes' 


phenomenon: after plugging, my Guest is failed to check with "ping", for a 
while, my display is light and load windows.


expect: my guest is running when i plug HDMI, it  seem that hot add a device. 

Re: [Qemu-devel] [PATCH 1/2] colo: compare the packet based on the tcp sequence number

[Mao Zhongyi]

On 12/04/2017 03:24 PM, Zhang Chen wrote:



On Mon, Dec 4, 2017 at 3:32 AM, Mao Zhongyi mailto:maozy.f...@cn.fujitsu.com>> wrote:



On 12/04/2017 09:41 AM, Zhang Chen wrote:



On Tue, Nov 28, 2017 at 8:04 PM, Mao Zhongyi mailto:maozy.f...@cn.fujitsu.com> >> wrote:

The primary and secondary guest has the same TCP stream, but the
the packet sizes are different due to the different 
fragmentation.

In the current impletation, compare the packet with the size of
payload, but packets of the same size and payload are very few,
so it triggers checkopint frequently, which leads to a very low
performance of the tcp packet comparison. In addtion, the method
of comparing the size of packet is not correct in itself.

like that:
We send this payload:
--
| header |1|2|3|4|5|6|7|8|9|0|
--

primary:
ppkt1:

| header |1|2|3|

ppkt2:

| header |4|5|6|7|8|9|0|


secondary:
spkt1:
--
| header |1|2|3|4|5|6|7|8|9|0|
--

In the original method, ppkt1 and ppkt2 are diffrent in size and
spkt1, so they can't compare and trigger the checkpoint.

I have tested FTP get 200M and 1G file many times, I found that
the performance was less than 1% of the native.

Now I reconstructed the comparison of TCP packets based on the
TCP sequence number. first of all, ppkt1 and spkt1 have the same
starting sequence number, so they can compare, even though their
length is different. And then ppkt1 with a smaller payload 
length
is used as the comparison length, if the payload is same, send
out the ppkt1 and record the offset(the length of ppkt1 payload)
in spkt1. The next comparison, ppkt2 and spkt1 can be compared
from the recorded position of spkt1.

like that:

| header |1|2|3| ppkt1
-|-|
 | |
-v-v--
| header |1|2|3|4|5|6|7|8|9|0| spkt1
---|\|
   | \offset |
  -v-v
  | header |4|5|6|7|8|9|0| ppkt2
  

In this way, the performance can reach native 20% in my multiple
tests.

Cc: Zhang Chen mailto:zhangc...@gmail.com> 
>>
Cc: Li Zhijian mailto:lizhij...@cn.fujitsu.com> 
>>
Cc: Jason Wang mailto:jasow...@redhat.com> 
>>

Reported-by: Zhang Chen mailto:zhangc...@gmail.com> 
>>
Signed-off-by: Mao Zhongyi mailto:maozy.f...@cn.fujitsu.com> >>
Signed-off-by: Li Zhijian mailto:lizhij...@cn.fujitsu.com> >>

---
 net/colo-compare.c | 300 
+
 net/colo.c |   8 ++
 net/colo.h |  14 +++
 3 files changed, 211 insertions(+), 111 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 1ce195f..0752e9f 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -38,6 +38,9 @@
 #define COMPARE_READ_LEN_MAX NET_BUFSIZE
 #define MAX_QUEUE_SIZE 1024

+#define COLO_COMPARE_FREE_PRIMARY 0x01
+#define COLO_COMPARE_FREE_SECONDARY   0x02
+
 /* TODO: Should be configurable */
 #define REGULAR_PACKET_CHECK_MS 3000

@@ -112,14 +115,31 @@ static gint seq_sorter(Packet *a, Packet *b, 
gpointer data)
 return ntohl(atcp->th_seq) - ntohl(btcp->th_seq);
 }

+static void fill_pkt_seq(void *data, uint32_t *max_ack)
+{
+Packet *pkt = data;

Re: [Qemu-devel] [PATCH 17/25] spapr: add a sPAPRXive object to the machine

[Cédric Le Goater]

On 12/04/2017 02:59 AM, David Gibson wrote:
> On Fri, Dec 01, 2017 at 09:10:24AM +0100, Cédric Le Goater wrote:
>> On 12/01/2017 05:14 AM, David Gibson wrote:
>>> On Thu, Nov 30, 2017 at 03:15:09PM +, Cédric Le Goater wrote:
 On 11/30/2017 05:55 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:47PM +0100, Cédric Le Goater wrote:
>> The XIVE object is designed to be always available, so it is created
>> unconditionally on newer machines.
>
> There doesn't actually seem to be anything dependent on machine
> version here.

 No. I thought that was too early in the patchset. This is handled 
 in the last patch with a 'xive_exploitation' bool which is set to 
 false on older machines. 

 But, nevertheless, the XIVE objects are always created even if not
 used. Something to discuss.
>>>
>>> That'll definitely break backwards migration, since the destination
>>> won't understand the (unused but still present) xive state it
>>> receives. 
>>
>> no because it's not sent. the vmstate 'needed' op of the sPAPRXive
>> object discards it :
>>
>> static bool vmstate_spapr_xive_needed(void *opaque)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>  
>> return spapr->xive_exploitation;
>> }
> 
> Ah, sorry, missed that.  Once we have negotiation we'll need to make
> sure the xive_exploitation bit is sent first, of course, but I'm
> pretty sure the machine state is already sent first.
> 
>>> So xives can only be created on new machine types. 
>>
>> That would be better I agree. I can probably use the 'xive_exploitation'
>> bool to condition its creation.
> 
> Hrm.  I'm less sure about that - I'm not sure the lifetimes line up.
> But I'd like to avoid creating them on earlier machine types, even if
> xive_exploitation can't do the trick.

Yes. I agree. I think we can work something out without introducing
too much complexity. The XIVE object is directly used by the
machine only to set/unset IRQ numbers. Otherwise, it is always 
conditioned by :

spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)

I think adding a couple of more tests on the 'xive_exploitation'
bool should work out for older machines.

Thanks,
 
C.

Re: [Qemu-devel] [PATCH 20/25] spapr: add device tree support for the XIVE interrupt mode

[David Gibson]

On Thu, Nov 23, 2017 at 02:29:50PM +0100, Cédric Le Goater wrote:
> The XIVE interface for the guest is described in the device tree under
> the "interrupt-controller" node. A couple of new properties are
> specific to XIVE :
> 
>  - "reg"
> 
>contains the base address and size of the thread interrupt
>managnement areas (TIMA), also called rings, for the User level and
>for the Guest OS level. Only the Guest OS level is taken into
>account today.
> 
>  - "ibm,xive-eq-sizes"
> 
>the size of the event queues. One cell per size supported, contains
>log2 of size, in ascending order.
> 
>  - "ibm,xive-lisn-ranges"
> 
>the interrupt numbers ranges assigned to the guest. These are
>allocated using a simple bitmap.
> 
> and also under the root node :
> 
>  - "ibm,plat-res-int-priorities"
> 
>contains a list of priorities that the hypervisor has reserved for
>its own use. Simulate ranges as defined by the PowerVM Hypervisor.
> 
> When the XIVE interrupt mode is activated after the CAS negotiation,
> the machine will perform a reboot to rebuild the device tree.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/spapr_xive_hcall.c  | 50 
> +
>  hw/ppc/spapr.c  |  7 ++-
>  hw/ppc/spapr_hcall.c|  6 ++
>  include/hw/ppc/spapr_xive.h |  2 ++
>  4 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c
> index 676fe0e2d5c7..60c6c9f4be8f 100644
> --- a/hw/intc/spapr_xive_hcall.c
> +++ b/hw/intc/spapr_xive_hcall.c
> @@ -883,3 +883,53 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
>  spapr_register_hypercall(H_INT_SYNC, h_int_sync);
>  spapr_register_hypercall(H_INT_RESET, h_int_reset);
>  }
> +
> +void spapr_xive_populate(sPAPRMachineState *spapr, int nr_servers,
> + void *fdt, uint32_t phandle)

Call it spapr_dt_xive() please, I'm trying to standardize on that
pattern for functions creating DT pieces.

> +{
> +sPAPRXive *xive = spapr->xive;
> +int node;
> +uint64_t timas[2 * 2];
> +uint32_t lisn_ranges[] = {
> +cpu_to_be32(0),
> +cpu_to_be32(nr_servers),
> +};
> +uint32_t eq_sizes[] = {
> +cpu_to_be32(12), /* 4K */
> +cpu_to_be32(16), /* 64K */
> +cpu_to_be32(21), /* 2M */
> +cpu_to_be32(24), /* 16M */
> +};
> +uint32_t plat_res_int_priorities[ARRAY_SIZE(reserved_priorities)];
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(plat_res_int_priorities); i++) {
> +plat_res_int_priorities[i] = cpu_to_be32(reserved_priorities[i]);
> +}
> +
> +/* Thread Interrupt Management Areas : User and OS */
> +for (i = 0; i < 2; i++) {
> +timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << 
> xive->tm_shift));
> +timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift);
> +}
> +
> +_FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));

You need a unit address here matching the reg property.

> +
> +_FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller"));

You don't need to set name properties explicitly for flattened trees.

> +_FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe"));
> +_FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas)));
> +
> +_FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe"));
> +_FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes,
> + sizeof(eq_sizes)));
> +_FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges,
> + sizeof(lisn_ranges)));
> +
> +/* For SLOF */
> +_FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
> +_FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
> +
> +/* top properties */
> +_FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
> + plat_res_int_priorities, 
> sizeof(plat_res_int_priorities)));
> +}
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8b15c0b500d0..3a62369883cc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1127,7 +1127,12 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>  
>  /* /interrupt controller */
> -spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
> +} else {
> +/* Populate device tree for XIVE */
> +spapr_xive_populate(spapr, xics_max_server_number(), fdt, 
> PHANDLE_XICP);
> +}
>  
>  ret = spapr_populate_memory(spapr, fdt);
>  if (ret < 0) {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index be22a6b2895f..e2a1665beee9 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1646,6 +1646,12 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *c

Re: [Qemu-devel] [PATCH 21/25] spapr: introduce a helper to map the XIVE memory regions

[David Gibson]

On Thu, Nov 23, 2017 at 02:29:51PM +0100, Cédric Le Goater wrote:
> When the XIVE interrupt mode is activated, the machine needs to expose
> to the guest the MMIO regions use by the controller :
> 
>   - Event State Buffer (ESB)
>   - Thread Interrupt Management Area (TIMA)
> 
> Migration will also need to reflect the current interrupt mode in use.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/intc/spapr_xive_hcall.c  | 14 ++
>  hw/ppc/spapr.c  |  5 +
>  include/hw/ppc/spapr_xive.h |  1 +
>  3 files changed, 20 insertions(+)
> 
> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c
> index 60c6c9f4be8f..ba217144878e 100644
> --- a/hw/intc/spapr_xive_hcall.c
> +++ b/hw/intc/spapr_xive_hcall.c
> @@ -933,3 +933,17 @@ void spapr_xive_populate(sPAPRMachineState *spapr, int 
> nr_servers,
>  _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
>   plat_res_int_priorities, 
> sizeof(plat_res_int_priorities)));
>  }
> +
> +void spapr_xive_mmio_map(sPAPRMachineState *spapr)
> +{
> +sPAPRXive *xive = spapr->xive;
> +
> +/* ESBs */
> +sysbus_mmio_map(SYS_BUS_DEVICE(xive), 0, xive->esb_base);
> +
> +/* Thread Management Interrupt Areas */
> +/* TODO: Only map the OS TIMA for the moment. Mapping the whole
> + * region needs some rework in the handlers */
> +sysbus_mmio_map(SYS_BUS_DEVICE(xive), 1,
> +xive->tm_base + (1 << xive->tm_shift));

You probably shouldn't be exposing the user TIMA in the DT if you're
only allowing the OS TIME to be mapped.

> +}
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3a62369883cc..734706c18cb3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1132,6 +1132,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  } else {
>  /* Populate device tree for XIVE */
>  spapr_xive_populate(spapr, xics_max_server_number(), fdt, 
> PHANDLE_XICP);
> +spapr_xive_mmio_map(spapr);

This doesn't belong here, spapr_build_fdt() should _just_ build the
fdt, not have side effects on the actual device state.

>  }
>  
>  ret = spapr_populate_memory(spapr, fdt);
> @@ -1613,6 +1614,10 @@ static int spapr_post_load(void *opaque, int 
> version_id)
>  }
>  }
>  
> +if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +spapr_xive_mmio_map(spapr);
> +}
> +
>  return err;
>  }
>  
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index f6d4bf26e06a..88355f7eb643 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -84,5 +84,6 @@ typedef struct sPAPRMachineState sPAPRMachineState;
>  void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>  void spapr_xive_populate(sPAPRMachineState *spapr, int nr_servers, void *fdt,
>   uint32_t phandle);
> +void spapr_xive_mmio_map(sPAPRMachineState *spapr);
>  
>  #endif /* PPC_SPAPR_XIVE_H */

-- 
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 23/25] spapr: toggle the ICP depending on the selected interrupt mode

[David Gibson]

On Thu, Nov 23, 2017 at 02:29:53PM +0100, Cédric Le Goater wrote:
> Each interrupt mode has its own specific interrupt presenter object,
> that we store under the CPU object, one for XICS and one for XIVE. The
> active presenter, corresponding to the current interrupt mode, is
> simply selected with a lookup on the children of the CPU.
> 
> Migration and CPU hotplug also need to reflect the current interrupt
> mode in use.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ppc/spapr.c  | 21 -
>  hw/ppc/spapr_cpu_core.c | 31 +++
>  include/hw/ppc/spapr_cpu_core.h |  1 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a91ec1c0751a..b7389dbdf5ca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1128,8 +1128,10 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  
>  /* /interrupt controller */
>  if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +spapr_cpu_core_set_icp(spapr->icp_type);
>  spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
>  } else {
> +spapr_cpu_core_set_icp(TYPE_SPAPR_XIVE_ICP);

Again you shouldn't have non-DT side-effects from spapr_build_fdt().

>  /* Populate device tree for XIVE */
>  spapr_xive_populate(spapr, xics_max_server_number(), fdt, 
> PHANDLE_XICP);
>  spapr_xive_mmio_map(spapr);
> @@ -1615,6 +1617,7 @@ static int spapr_post_load(void *opaque, int version_id)
>  }
>  
>  if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +spapr_cpu_core_set_icp(TYPE_SPAPR_XIVE_ICP);
>  spapr_xive_mmio_map(spapr);
>  }
>  
> @@ -3610,7 +3613,7 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int 
> vcpu_id)
>  Object *spapr_icp_create(sPAPRMachineState *spapr, CPUState *cs, Error 
> **errp)
>  {
>  Error *local_err = NULL;
> -Object *obj;
> +Object *obj, *obj_xive;
>  
>  obj = icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err);
>  if (local_err) {
> @@ -3618,6 +3621,22 @@ Object *spapr_icp_create(sPAPRMachineState *spapr, 
> CPUState *cs, Error **errp)
>  return NULL;
>  }
>  
> +/* Add a XIVE interrupt presenter. The machine will switch the CPU
> + * ICP depending on the interrupt model negotiated at CAS time.
> + */
> +obj_xive = icp_create(cs, TYPE_SPAPR_XIVE_ICP, XICS_FABRIC(spapr),
> +  &local_err);

You shouldn't be using icp_create() a xics function, for xive.

> +if (local_err) {
> +object_unparent(obj);
> +error_propagate(errp, local_err);
> +return NULL;
> +}
> +
> +/* when hotplugged, the CPU should have the correct ICP */
> +if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +return obj_xive;
> +}
> +
>  return obj;
>  }
>  
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 61a9850e688b..b0e39270f262 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -257,3 +257,34 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindICPArgs {
> +const char *icp_type;
> +Object *icp;
> +} ForeachFindICPArgs;
> +
> +static int spapr_cpu_core_find_icp(Object *child, void *opaque)
> +{
> +ForeachFindICPArgs *args = opaque;
> +
> +if (object_dynamic_cast(child, args->icp_type)) {
> +args->icp = child;
> +}
> +
> +return args->icp != NULL;
> +}
> +
> +void spapr_cpu_core_set_icp(const char *icp_type)
> +{
> +CPUState *cs;
> +
> +CPU_FOREACH(cs) {
> +ForeachFindICPArgs args = { icp_type, NULL };
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +object_child_foreach(OBJECT(cs), spapr_cpu_core_find_icp, &args);
> +g_assert(args.icp);
> +
> +cpu->intc = args.icp;
> +}
> +}
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index f2d48d6a6786..a657dfb8863c 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -38,4 +38,5 @@ typedef struct sPAPRCPUCoreClass {
>  } sPAPRCPUCoreClass;
>  
>  const char *spapr_get_cpu_core_type(const char *cpu_type);
> +void spapr_cpu_core_set_icp(const char *icp_type);
>  #endif

-- 
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 22/25] spapr: add XIVE support to spapr_irq_get_qirq()

[David Gibson]

On Thu, Nov 23, 2017 at 02:29:52PM +0100, Cédric Le Goater wrote:
> The XIVE object has its own set of qirqs which is to be used when the
> XIVE interrupt mode is activated.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ppc/spapr.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 734706c18cb3..a91ec1c0751a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3746,8 +3746,12 @@ qemu_irq spapr_irq_get_qirq(sPAPRMachineState *spapr, 
> int irq)
>  {
>  ICSState *ics = spapr->ics;
>  
> -if (ics_valid_irq(ics, irq)) {
> -return ics->qirqs[irq - ics->offset];
> +if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +return spapr->xive->qirqs[irq];

You should have a xive helper function for this - spapr code shouldn't
be reaching into the internal XIVE structure.

> +} else {
> +if (ics_valid_irq(ics, irq)) {
> +return ics->qirqs[irq - ics->offset];
> +}
>  }
>  
>  return NULL;

-- 
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 17/25] spapr: add a sPAPRXive object to the machine

[David Gibson]

On Mon, Dec 04, 2017 at 09:32:00AM +0100, Cédric Le Goater wrote:
> On 12/04/2017 02:59 AM, David Gibson wrote:
> > On Fri, Dec 01, 2017 at 09:10:24AM +0100, Cédric Le Goater wrote:
> >> On 12/01/2017 05:14 AM, David Gibson wrote:
> >>> On Thu, Nov 30, 2017 at 03:15:09PM +, Cédric Le Goater wrote:
>  On 11/30/2017 05:55 AM, David Gibson wrote:
> > On Thu, Nov 23, 2017 at 02:29:47PM +0100, Cédric Le Goater wrote:
> >> The XIVE object is designed to be always available, so it is created
> >> unconditionally on newer machines.
> >
> > There doesn't actually seem to be anything dependent on machine
> > version here.
> 
>  No. I thought that was too early in the patchset. This is handled 
>  in the last patch with a 'xive_exploitation' bool which is set to 
>  false on older machines. 
> 
>  But, nevertheless, the XIVE objects are always created even if not
>  used. Something to discuss.
> >>>
> >>> That'll definitely break backwards migration, since the destination
> >>> won't understand the (unused but still present) xive state it
> >>> receives. 
> >>
> >> no because it's not sent. the vmstate 'needed' op of the sPAPRXive
> >> object discards it :
> >>
> >> static bool vmstate_spapr_xive_needed(void *opaque)
> >> {
> >> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >>  
> >> return spapr->xive_exploitation;
> >> }
> > 
> > Ah, sorry, missed that.  Once we have negotiation we'll need to make
> > sure the xive_exploitation bit is sent first, of course, but I'm
> > pretty sure the machine state is already sent first.
> > 
> >>> So xives can only be created on new machine types. 
> >>
> >> That would be better I agree. I can probably use the 'xive_exploitation'
> >> bool to condition its creation.
> > 
> > Hrm.  I'm less sure about that - I'm not sure the lifetimes line up.
> > But I'd like to avoid creating them on earlier machine types, even if
> > xive_exploitation can't do the trick.
> 
> Yes. I agree. I think we can work something out without introducing
> too much complexity. The XIVE object is directly used by the
> machine only to set/unset IRQ numbers. Otherwise, it is always 
> conditioned by :
> 
> spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)
> 
> I think adding a couple of more tests on the 'xive_exploitation'
> bool should work out for older machines.

Ok.  If not you can always add a "xive_possible" flag to the MachineClass.

-- 
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 v4 12/20] qed: Switch to .bdrv_co_block_status()

[Vladimir Sementsov-Ogievskiy]

01.12.2017 02:17, Eric Blake wrote:

On 11/30/2017 04:27 AM, Vladimir Sementsov-Ogievskiy wrote:

12.10.2017 21:59, Eric Blake wrote:

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the qed driver accordingly, taking the opportunity
to inline qed_is_allocated_cb() into its lone caller (the callback
used to be important, until we switched qed to coroutines). There is
no intent to optimize based on the want_zero flag for this format.

Signed-off-by: Eric Blake 




  {
  BDRVQEDState *s = bs->opaque;
-    size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE;
-    QEDIsAllocatedCB cb = {
-    .bs = bs,
-    .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE,
-    .status = BDRV_BLOCK_OFFSET_MASK,
-    .pnum = pnum,
-    .file = file,
-    };
+    size_t len;


size_t len = bytes;



Or rather, size_t len = MIN(bytes, SIZE_MAX); thanks to 32-bit platforms.


ok, this is even better




with that:
Reviewed-by: Vladimir Sementsov-Ogievskiy 


+    int status;
  QEDRequest request = { .l2_table = NULL };
  uint64_t offset;
  int ret;

  qemu_co_mutex_lock(&s->table_lock);
-    ret = qed_find_cluster(s, &request, cb.pos, &len, &offset);
-    qed_is_allocated_cb(&cb, ret, offset, len);
+    ret = qed_find_cluster(s, &request, pos, &len, &offset);


len is in-out parameter, you can't use it uninitialized.


Good catch.




--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH RFC 0/2] s390x: cut down on unattached devices

[Cornelia Huck]

On Fri, 1 Dec 2017 15:41:21 +0100
Halil Pasic  wrote:

> On 11/28/2017 04:21 PM, Halil Pasic wrote:
> [..]
> >>> Otherwise at first glance both patches seem sane.  
> >>
> >> Can I count this as an ack, or do you plan to do more review?
> >>  
> > 
> > Yes I was planning to give it another look. And I do already
> > have questions. Isn't the QOM composition tree API? I mean
> > let's assume the QMP commands working on this tree are not completely
> > useless. How is client code (management software) supposed to work,
> > assumed it can rely on paths of e.g. properties being stable. Just
> > imagine we had this default-cssid property (for the sake of the
> > argument, not like we want it) on the css bridge.  
> 
> Ping! I would like to get this clarified before proceeding with reviewing
> this series.

[It might be helpful to not drop cc:s.]

I don't think we really want a static tree. As long as the devices are
locateable, it should be fine.

> 
> > 
> > Now if the composition tree is API then these can only be bug fixes
> > (IMHO).
> > 
> > There are also other oddities I've spotted. My idea was to put
> > this composition tree discussion on hold until the vfio-ccw stuff
> > is sorted out. I would certainly like to build a better understanding.
> > 
> > Halil
> >   
> 
> [..]
> 
> 

Re: [Qemu-devel] [Bug 1735384] Re: OpenJDK JVM segfaults on qemu-sh4 (regression)

[Alex Bennée]

Thomas Huth  writes:

> On 01.12.2017 00:25, John Paul Adrian Glaubitz wrote:
>> The offending commit is:
>> 
>> d25f2a72272b9ffe0d06710d6217d1169bc2cc7d is the first bad commit
>> commit d25f2a72272b9ffe0d06710d6217d1169bc2cc7d
>> Author: Alex Bennée 
>> Date:   Mon Nov 13 13:55:27 2017 +
>> 
>> accel/tcg/translate-all: expand cpu_restore_state addr check
>> 
>> We are still seeing signals during translation time when we walk over
>> a page protection boundary. This expands the check to ensure the host
>> PC is inside the code generation buffer. The original suggestion was
>> to check versus tcg_ctx.code_gen_ptr but as we now segment the
>> translation buffer we have to settle for just a general check for
>> being inside.
>> 
>> I've also fixed up the declaration to make it clear it can deal with
>> invalid addresses. A later patch will fix up the call sites.
>> 
>> Signed-off-by: Alex Bennée 
>> Reported-by: Peter Maydell 
>> Reviewed-by: Laurent Vivier 
>> Reviewed-by: Richard Henderson 
>> Message-id: 20171108153245.20740-2-alex.ben...@linaro.org
>> Suggested-by: Paolo Bonzini 
>> Cc: Richard Henderson 
>> Tested-by: Peter Maydell 
>> Signed-off-by: Peter Maydell 
>> 
>> :04 04 da50c4c43089d3ee7d1e9ad50d3c9036114e5f11 
>> cd6a0dcaa1d284fe5439f6f3b61547d4b0662768 M  accel
>> :04 04 c294a7c102d27295f8d81cc06b5d4d17357440ad 
>> 5a1268b7634f69f0806f22161ec7d6a1a26c8812 M  include
>> 
>> Reverting the commit resolves the issue.
>> 
>
> Alex, any ideas what might be wrong here?

It's hard to imagine a scenario where taking the tb_lock() for resolving
something that will fail is going to be an improvement. However maybe
there is a subtle difference with sh4's javavm implementation.

A backtrace QEMU after the segv would be useful here.

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH 08/17] e500: additional CCSR registers

[David Gibson]

On Sun, Nov 26, 2017 at 03:59:06PM -0600, Michael Davidsaver wrote:
> Add CCSRBAR to allow CCSR region to be relocated.
> 
> Guest memory size introspection via RAM config
> registers.
> 
> Dummy RAM error controls.
> 
> Clock introspection via Power on Reset PLL
> Status Register.
> 
> Signed-off-by: Michael Davidsaver 

Applied to ppc-for-2.12.

> 
> ccsrbase also update iack
> ---
>  hw/ppc/e500.c  |  5 ++-
>  hw/ppc/e500_ccsr.c | 93 
> --
>  2 files changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index b90f4231a6..e22919f4f1 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -51,7 +51,9 @@
>  
>  #define RAM_SIZES_ALIGN(64UL << 20)
>  
> -/* TODO: parameterize */
> +/* TODO: parameterize
> + * Some CCSR offsets duplicated in e500_ccsr.c
> + */
>  #define MPC8544_CCSRBAR_SIZE   0x0010ULL
>  #define MPC8544_MPIC_REGS_OFFSET   0x4ULL
>  #define MPC8544_MSI_REGS_OFFSET   0x41600ULL
> @@ -856,6 +858,7 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>  object_property_add_child(qdev_get_machine(), "e500-ccsr",
>OBJECT(dev), NULL);
>  qdev_prop_set_uint32(dev, "base", params->ccsrbar_base);
> +qdev_prop_set_uint32(dev, "ram-size", ram_size);
>  qdev_init_nofail(dev);
>  ccsr_addr_space = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
>  
> diff --git a/hw/ppc/e500_ccsr.c b/hw/ppc/e500_ccsr.c
> index 1b586c3f42..9400d7cf13 100644
> --- a/hw/ppc/e500_ccsr.c
> +++ b/hw/ppc/e500_ccsr.c
> @@ -30,13 +30,26 @@
>  #include "hw/sysbus.h"
>  
>  /* E500_ denotes registers common to all */
> +/* Some CCSR offsets duplicated in e500.c */
>  
> +#define E500_CCSRBAR (0)
> +
> +#define E500_CS0_BNDS(0x2000)
> +
> +#define E500_CS0_CONFIG  (0x2080)
> +
> +#define E500_ERR_DETECT  (0x2e40)
> +#define E500_ERR_DISABLE (0x2e44)
> +
> +#define E500_PORPLLSR(0xE)
>  #define E500_PVR (0xE00A0)
>  #define E500_SVR (0xE00A4)
>  
>  #define MPC8544_RSTCR   (0xE00B0)
>  #define MPC8544_RSTCR_RESET  (0x02)
>  
> +#define E500_MPIC_OFFSET   (0x4ULL)
> +
>  typedef struct {
>  /*< private >*/
>  SysBusDevice parent_obj;
> @@ -44,19 +57,59 @@ typedef struct {
>  
>  MemoryRegion iomem;
>  
> -uint32_t defbase;
> +uint32_t defbase, base;
> +uint32_t ram_size;
> +uint32_t merrd;
> +
> +uint32_t porpllsr;
> +
> +DeviceState *pic;
>  } CCSRState;
>  
>  #define TYPE_E500_CCSR "e500-ccsr"
>  #define E500_CCSR(obj) OBJECT_CHECK(CCSRState, (obj), TYPE_E500_CCSR)
>  
> +/* call after changing CCSRState::base */
> +static void e500_ccsr_post_move(CCSRState *ccsr)
> +{
> +CPUState *cs;
> +
> +CPU_FOREACH(cs) {
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +CPUPPCState *env = &cpu->env;
> +
> +env->mpic_iack = ccsr->base +
> + E500_MPIC_OFFSET + 0xa0;
> +}
> +
> +sysbus_mmio_map(SYS_BUS_DEVICE(ccsr), 0, ccsr->base);
> +}
> +
>  static uint64_t e500_ccsr_read(void *opaque, hwaddr addr,
>unsigned size)
>  {
> +CCSRState *ccsr = opaque;
>  PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>  CPUPPCState *env = &cpu->env;
>  
>  switch (addr) {
> +case E500_CCSRBAR:
> +return ccsr->base >> 12;
> +case E500_CS0_BNDS:
> +/* we model all RAM in a single chip with addresses [0, ram_size) */
> +return (ccsr->ram_size - 1) >> 24;
> +case E500_CS0_CONFIG:
> +return 1 << 31;
> +case E500_ERR_DETECT:
> +return 0; /* (errors not modeled) */
> +case E500_ERR_DISABLE:
> +return ccsr->merrd;
> +case E500_PORPLLSR:
> +if (!ccsr->porpllsr) {
> +qemu_log_mask(LOG_UNIMP,
> +  "Machine does not provide valid PORPLLSR\n");
> +}
> +return ccsr->porpllsr;
>  case E500_PVR:
>  return env->spr[SPR_PVR];
>  case E500_SVR:
> @@ -72,10 +125,22 @@ static uint64_t e500_ccsr_read(void *opaque, hwaddr addr,
>  static void e500_ccsr_write(void *opaque, hwaddr addr,
> uint64_t value, unsigned size)
>  {
> +CCSRState *ccsr = opaque;
>  PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>  CPUPPCState *env = &cpu->env;
>  uint32_t svr = env->spr[SPR_E500_SVR] >> 16;
>  
> +switch (addr) {
> +case E500_CCSRBAR:
> +value &= 0x000fff00;
> +ccsr->base = value << 12;
> +e500_ccsr_post_move(ccsr);
> +return;
> +case E500_ERR_DISABLE:
> +ccsr->merrd = value & 0xd;
> +return;
> +}
> +
>  switch (svr) {
>  case 0: /* generic.  assumed to be mpc8544ds or e500plat board */
>  case 0x8034: /* mpc8544 */
> @@ -104,11 +169,20 @@ static const MemoryRegionOps e500_ccsr_ops = {
>  }
>  };
>  
> +static int e500_ccsr_post_load(void *opaque, int version_id)
> 

Re: [Qemu-devel] [PULL 0/7] pc, pci, virtio: fixes for rc3

[Peter Maydell]

On 3 December 2017 at 04:56, Michael S. Tsirkin  wrote:
> On Fri, Dec 01, 2017 at 06:05:25PM +, Peter Maydell wrote:
>> On 1 December 2017 at 17:08, Michael S. Tsirkin  wrote:
>> > Chao Gao (1):
>> >   i386/msi: Correct mask of destination ID in MSI address
>> >
>> > Greg Kurz (1):
>> >   vhost: fix error check in vhost_verify_ring_mappings()
>> >
>> > Igor Mammedov (1):
>> >   pc: fix crash on attempted cpu unplug
>> >
>> > Marc-André Lureau (1):
>> >   dump-guest-memory.py: fix No symbol "vmcoreinfo_find"
>> >
>> > Maxime Coquelin (2):
>> >   virtio: Add queue interface to restore avail index from vring used 
>> > index
>> >   vhost: restore avail index from vring used index on disconnection
>> >
>> > Prasad J Pandit (1):
>> >   virtio: check VirtQueue Vring object is set
>>
>> Are any of these so important that we would absolutely refuse
>> to release without the fixes (ie they justify rolling an rc4
>> that we would otherwise not have needed) ?

> The msi one is less important it just happened to be queued a while ago
> and I didn't want to rebase all testing. Others are crashers but they
> don't affect everyone. So I wouldn't be sure, but there's also a
> security fix in there, so yes, I suspect we are better off with rc4, and
> if we do I think including others is justified (except maybe the msi
> one, if you feel strongly I'll rebase and drop it).

The bar has to be set quite high here, because if it turns out
that there are problems with a bug fix then we are out of time
to rework or revert it. And the more fixes we throw in at the
last minute, even if they're individually simple, the more
likely that one of them turns out to have unexpected consequences.

So: were any of these bugs present in the 2.10 release? If so,
that strongly argues for not trying to fix them at this point.

With all of these patches plus David Gibson's, that would be
10 new patches in rc4. That is definitely more than would be
ideal.

thanks
-- PMM

Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?

[Stefan Hajnoczi]

On Fri, Dec 01, 2017 at 01:27:37PM -0600, Eric Blake wrote:
> On 12/01/2017 11:03 AM, Paolo Bonzini wrote:
> > Just my 2 cents on the language topic, as in general I agree completely
> > with Stefan.
> > 
> 
> > Luckily, several benefits don't require a full rewrite or language switch:
> > 
> > - readability from RAII-style code.  If this is important enough, we
> > could actually use GCC __attribute__((cleanup)) or, heaven forbid,
> > slowly introduce C++ in QEMU's code base.
> 
> This one is cool, and supported by both gcc and clang (and we really don't
> have any example of anyone clamoring for a different compiler). For example,
> here's what I recently added to nbdkit to make it MUCH easier to properly
> remember to unlock a mutex on all exit paths from a scope:
> 
> https://github.com/libguestfs/nbdkit/commit/0f24d66648
> https://github.com/libguestfs/nbdkit/commit/b5ab4835e1

Yes, please!  __attribute__((cleanup)) is great and we should use it in
QEMU.

I haven't looked at compiler support though... :)

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 0/3] ppc-for-2.11 queue 20171204

[Peter Maydell]

On 4 December 2017 at 03:47, David Gibson  wrote:
> The following changes since commit c11d61271b9e6e7a1f0479ef1ca8fb55fa457a62:
>
>   Update version for v2.11.0-rc3 release (2017-11-29 17:59:34 +)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.11-20171204
>
> for you to fetch changes up to 768a20f3a491ed4afce73ebb65347d55251c0ebd:
>
>   spapr: Include "pre-plugged" DIMMS in ram size calculation at reset 
> (2017-12-04 11:31:22 +1100)
>
> 
> ppc patch queue 2017-12-04
>
> We are, alas, not yet to the bottom of ppc bugs.  This pull request
> fixes several more.  I believe they're important enough to include in
> 2.11. despite the late date.
>
> 
> David Gibson (1):
>   spapr: Include "pre-plugged" DIMMS in ram size calculation at reset
>
> Kurban Mallachiev (1):
>   target-ppc: Don't invalidate non-supported msr bits
>
> Laurent Vivier (1):
>   pseries: fix TCG migration

So my question here is about the same as to mst: are these fixes all
really vital? Were any of the bugs they fix present in 2.10 ?

(A +7-4 diffstat is not too difficult to justify though.)

thanks
-- PMM

[Qemu-devel] [PATCH v2] pci: removed the is_express field since a uniform interface was inserted

[Yoni Bettan]

* according to Eduardo Habkost's commit
  fd3b02c8896d597dd8b9e053dec579cf0386aee1

* since all PCIEs now implement INTERFACE_PCIE_DEVICE we
  don't need this field anymore

* Devices that where only INTERFACE_PCIE_DEVICE (is_express == 1)
  or
  devices that where only INTERFACE_CONVENTIONAL_PCI_DEVICE (is_express 
== 0)
  where not affected by the change

  The only devices that were affected are those that are hybrid and also
  had (is_express == 1) - therefor only:
- hw/vfio/pci.c
- hw/usb/hcd-xhci.c

  For both i made sure that pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS

Signed-off-by: Yoni Bettan 
---
 docs/pcie_pci_bridge.txt   |  2 +-
 hw/block/nvme.c|  1 -
 hw/net/e1000e.c|  1 -
 hw/pci-bridge/pcie_pci_bridge.c|  1 -
 hw/pci-bridge/pcie_root_port.c |  1 -
 hw/pci-bridge/xio3130_downstream.c |  1 -
 hw/pci-bridge/xio3130_upstream.c   |  1 -
 hw/pci-host/xilinx-pcie.c  |  1 -
 hw/pci/pci.c   |  4 +++-
 hw/scsi/megasas.c  |  4 
 hw/usb/hcd-xhci.c  | 28 ++--
 hw/vfio/pci.c  | 37 +++--
 include/hw/pci/pci.h   |  3 ---
 include/hw/usb.h   |  1 +
 14 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/docs/pcie_pci_bridge.txt b/docs/pcie_pci_bridge.txt
index 5a4203f97c..55f833ec10 100644
--- a/docs/pcie_pci_bridge.txt
+++ b/docs/pcie_pci_bridge.txt
@@ -110,5 +110,5 @@ To enable device hot-plug into the bridge on Linux there're 
3 ways:
 Implementation
 ==
 The PCIE-PCI bridge is based on PCI-PCI bridge, but also accumulates PCI 
Express
-features as a PCI Express device (is_express=1).
+features as a PCI Express device (pci_dev->cap_present & QEMU_PCI_CAP_EXPRESS 
!= 0).
 
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 441e21ed1f..9325bc0911 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1087,7 +1087,6 @@ static void nvme_class_init(ObjectClass *oc, void *data)
 pc->vendor_id = PCI_VENDOR_ID_INTEL;
 pc->device_id = 0x5845;
 pc->revision = 2;
-pc->is_express = 1;
 
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 dc->desc = "Non-Volatile Memory Express";
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index f1af279e8d..c360f0d8c9 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -675,7 +675,6 @@ static void e1000e_class_init(ObjectClass *class, void 
*data)
 c->revision = 0;
 c->romfile = "efi-e1000e.rom";
 c->class_id = PCI_CLASS_NETWORK_ETHERNET;
-c->is_express = 1;
 
 dc->desc = "Intel 82574L GbE Controller";
 dc->reset = e1000e_qdev_reset;
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index a4d827c99d..b7d9ebbec2 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -169,7 +169,6 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
-k->is_express = 1;
 k->is_bridge = 1;
 k->vendor_id = PCI_VENDOR_ID_REDHAT;
 k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE;
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 9b6e4ce512..45f9e8cd4a 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -145,7 +145,6 @@ static void rp_class_init(ObjectClass *klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->is_express = 1;
 k->is_bridge = 1;
 k->config_write = rp_write_config;
 k->realize = rp_realize;
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index 1e09d2afb7..613a0d6bb7 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -177,7 +177,6 @@ static void xio3130_downstream_class_init(ObjectClass 
*klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->is_express = 1;
 k->is_bridge = 1;
 k->config_write = xio3130_downstream_write_config;
 k->realize = xio3130_downstream_realize;
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 227997ce46..d4645bddee 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -148,7 +148,6 @@ static void xio3130_upstream_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->is_express = 1;
 k->is_bridge = 1;
 k->config_write = xio3130_upstream_write_config;
 k->realize = xio3130_upstream_realize;
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 7659253090..a4ca3ba30f 100

Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?

[Peter Maydell]

On 4 December 2017 at 10:16, Stefan Hajnoczi  wrote:
> Yes, please!  __attribute__((cleanup)) is great and we should use it in
> QEMU.
>
> I haven't looked at compiler support though... :)

Looks like it came in in gcc 3.6, which is earlier than our earliest
required version. For clang, it's harder to say because the clang
documentation doesn't mention it at all, even though they actually
support it AFAICT from googling.

thanks
-- PMM

Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?

[Stefan Hajnoczi]

On Fri, Dec 01, 2017 at 07:03:15PM +, Peter Maydell wrote:
> On 1 December 2017 at 17:03, Paolo Bonzini  wrote:
> > Same here.  Just like fixing the C code provides a good foundation for a
> > language switch, some more battle-tested code could be converted from
> > QEMU to Rust, in order to get familiar with it and probe whether the
> > benefits are real.  Maybe the memory API could be a good candidate; it
> > certainly would benefit from generics.
> 
> At the moment I feel like betting the future of the project on
> Rust would be quite a courageous decision. On the other hand
> it might be interesting to look at prototyping to see what
> benefits it might bring. (One candidate I had in mind was the
> device API -- given that we have had quite a few buffer overruns
> in device code, converting some of the device models to Rust
> might cut off that particular security issue.)
> 
> I agree with you that trying to tackle conversion without at
> least one developer with reasonably-expert Rust knowledge is
> likely to be fraught, though -- what you really need is to be
> able to design APIs which are idiomatic for the language, rather
> than the kind of thing you'd write in C but transliterated across...

My main concern beyond lack of Rust experience is that quite a bit of
glue code is necessary to mix Rust and C:

  https://doc.rust-lang.org/book/first-edition/ffi.html

You need to redefine structs and functions in Rust.  To make the C
functions easily callable from Rust without unsafe code, you then need
to write Rust wrapper functions.

This means a lot of boilerplate and duplication.  It makes documenting
code harder.

IMO it will be very hard to replace a component in the QEMU codebase
with Rust code unless we're willing to put up with a lot of boilerplate
and duplication.

Where Rust makes a lot of sense to me is for new programs like
qemu-pr-helper, vhost-user programs, etc.  They have a clearly-defined
boundary (command-line, socket protocol, etc) where switching to Rust
doesn't require redeclaring a bunch of existing C interfaces.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] spapr: fix LSI interrupt specifiers in the device tree

[Greg Kurz]

On Sun, 3 Dec 2017 10:37:35 +1100
David Gibson  wrote:

> On Sat, Dec 02, 2017 at 08:30:11PM +0100, Greg Kurz wrote:
> > PAPR 2.7 C.6.9.1.2 describes the "#interrupt-cells" property of the
> > PowerPC External Interrupt Source Controller node as follows:
> > 
> > “#interrupt-cells”
> > 
> >   Standard property name to define the number of cells in an interrupt-
> >   specifier within an interrupt domain.
> > 
> >   prop-encoded-array: An integer, encoded as with encode-int, that denotes
> >   the number of cells required to represent an interrupt specifier in its
> >   child nodes.
> > 
> >   The value of this property for the PowerPC External Interrupt option shall
> >   be 2. Thus all interrupt specifiers (as used in the standard “interrupts”
> >   property) shall consist of two cells, each containing an integer encoded
> >   as with encode-int. The first integer represents the interrupt number the
> >   second integer is the trigger code: 0 for edge triggered, 1 for level
> >   triggered.
> > 
> > This patch adds a second cell to the interrupt specifier stored in the
> > "interrupts" property of PCI device nodes. This property only exists if
> > the Interrupt Pin register is set, ie, the interrupt is level, the extra
> > cell is hence set to 1.  
> 
> Nack.  This format of interrupt specifier is only needed for things
> wired directly to the xics.  The PCI INTx interrupts aren't - they go
> through the interrupt nexus in the PHB.  The interrupt-map is intended
> to remap the simple 1,2,3,4 for INTA..INTD to xics interrupt specifiers.
> 

Indeed you're right... and the “#interrupt-cells” property of the PHB
is set to 1, ie, the interrupt specifier format in the child PCI isn't
expected to have an extra cell... This should have rung a bell :)

> > This also fixes the interrupt specifiers in the "interrupt-map" property
> > of the PHB node, that were setting the second cell to 8 (confusion with
> > IRQ_TYPE_LEVEL_LOW ?) instead of 1.  
> 
> Fixing that is correct, though I think.  As might be the changes in
> other places I'll have to check.
> 
> > While here, let's introduce defines for the interrupt specifier trigger
> > code, and patch other users in spapr.
> > 
> > Signed-off-by: Greg Kurz   
> 
> 
> > ---
> > 
> > This fixes /proc/interrupts in linux guests where LSIs appear as
> > Edge instead of Level.
> > ---
> >  hw/ppc/spapr_events.c  |2 +-
> >  hw/ppc/spapr_pci.c |4 +++-
> >  hw/ppc/spapr_vio.c |3 ++-
> >  include/hw/ppc/spapr.h |3 +++
> >  4 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index e377fc7ddea2..4bcb98f948ea 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -283,7 +283,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void 
> > *fdt)
> >  }
> >  
> >  interrupts[0] = cpu_to_be32(source->irq);
> > -interrupts[1] = 0;
> > +interrupts[1] = SPAPR_DT_INTERRUPT_IDENTIFIER_EDGE;
> >  
> >  _FDT(node_offset = fdt_add_subnode(fdt, event_sources, 
> > source_name));
> >  _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 5a3122a9f9f9..91fedbf0929c 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1231,6 +1231,8 @@ static void spapr_populate_pci_child_dt(PCIDevice 
> > *dev, void *fdt, int offset,
> >  if (pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)) {
> >  _FDT(fdt_setprop_cell(fdt, offset, "interrupts",
> >   pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1)));
> > +_FDT(fdt_appendprop_cell(fdt, offset, "interrupts",
> > + SPAPR_DT_INTERRUPT_IDENTIFIER_LEVEL));
> >  }
> >  
> >  if (!is_bridge) {
> > @@ -2122,7 +2124,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> >  irqmap[3] = cpu_to_be32(j+1);
> >  irqmap[4] = cpu_to_be32(xics_phandle);
> >  irqmap[5] = cpu_to_be32(phb->lsi_table[lsi_num].irq);
> > -irqmap[6] = cpu_to_be32(0x8);
> > +irqmap[6] = cpu_to_be32(SPAPR_DT_INTERRUPT_IDENTIFIER_LEVEL);
> >  }
> >  }
> >  /* Write interrupt map */
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index ea3bc8bd9e21..29a17651a17c 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -126,7 +126,8 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
> >  }
> >  
> >  if (dev->irq) {
> > -uint32_t ints_prop[] = {cpu_to_be32(dev->irq), 0};
> > +uint32_t ints_prop[] = { cpu_to_be32(dev->irq),
> > + SPAPR_DT_INTERRUPT_IDENTIFIER_EDGE };
> >  
> >  ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
> >sizeof(ints_prop));
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 9d21ca9bde3a..8f6298bde59b 100644
> > --- a/include/hw/

[Qemu-devel] CFP deadline extended for FOSDEM18 Virt & IaaS Devroom

[Stefan Hajnoczi]

The call for papers for the Virtualization and IaaS Devroom at
FOSDEM18 is extended until December 7 midnight UTC. All other dates
remain the same as you can read below.

On behalf of oVirt and the Xen Project, we are excited to announce that the
call for proposals is now open for the Virtualization & IaaS devroom at the
upcoming FOSDEM 2018, to be hosted on February 3 and 4, 2017.

This year will mark FOSDEM’s 18th anniversary as one of the longest-running
free and open source software developer events, attracting thousands of
developers and users from all over the world. FOSDEM will be held once
again in Brussels, Belgium, on February 3 & 4, 2018.

This devroom is a collaborative effort, and is organized by dedicated folks
from projects such as OpenStack, Xen Project,, oVirt, QEMU, and
Foreman. We would like to invite all those who are involved in these fields
to submit your proposals by December 1st, 2017.

About the Devroom

The Virtualization & IaaS devroom will feature session topics such as open
source hypervisors and virtual machine managers such as Xen Project, KVM,
bhyve, and VirtualBox, and Infrastructure-as-a-Service projects such as
Apache CloudStack, OpenStack, oVirt, QEMU, OpenNebula, and Ganeti.

This devroom will host presentations that focus on topics of shared
interest, such as KVM; libvirt; shared storage; virtualized networking;
cloud security; clustering and high availability; interfacing with multiple
hypervisors; hyperconverged deployments; and scaling across hundreds or
thousands of servers.

Presentations in this devroom will be aimed at developers working on these
platforms who are looking to collaborate and improve shared infrastructure
or solve common problems. We seek topics that encourage dialog between
projects and continued work post-FOSDEM.

Important Dates

Submission deadline: 07 December 2017
Acceptance notifications: 14 December 2017
Final schedule announcement: 21 December 2017
Devroom: 03 and 04 February 2018 (two days- different rooms)

Submit Your Proposal

All submissions must be made via the Pentabarf event planning site[1]. If
you have not used Pentabarf before, you will need to create an account. If
you submitted proposals for FOSDEM in previous years, you can use your
existing account.

After creating the account, select Create Event to start the submission
process. Make sure to select Virtualization and IaaS devroom from the Track
list. Please fill out all the required fields, and provide a meaningful
abstract and description of your proposed session.

Submission Guidelines

We expect more proposals than we can possibly accept, so it is vitally
important that you submit your proposal on or before the deadline. Late
submissions are unlikely to be considered.

All presentation slots are 45 minutes, with 35 minutes planned for
presentations, and 10 minutes for Q&A.

All presentations will be recorded and made available under Creative
Commons licenses. In the Submission notes field, please indicate that you
agree that your presentation will be licensed under the CC-By-SA-4.0 or
CC-By-4.0 license and that you agree to have your presentation recorded.
For example:

"If my presentation is accepted for FOSDEM, I hereby agree to license all
recordings, slides, and other associated materials under the Creative
Commons Attribution Share-Alike 4.0 International License. Sincerely,
."

In the Submission notes field, please also confirm that if your talk is
accepted, you will be able to attend FOSDEM and deliver your presentation.
We will not consider proposals from prospective speakers who are unsure
whether they will be able to secure funds for travel and lodging to attend
FOSDEM. (Sadly, we are not able to offer travel funding for prospective
speakers.)

Speaker Mentoring Program

As a part of the rising efforts to grow our communities and encourage a
diverse and inclusive conference ecosystem, we're happy to announce that
we'll be offering mentoring for new speakers. Our mentors can help you with
tasks such as reviewing your abstract, reviewing your presentation outline
or slides, or practicing your talk with you.

You may apply to the mentoring program as a newcomer speaker if you:

Never presented before or
Presented only lightning talks or
Presented full-length talks at small meetups (<50 ppl)

Submission Guidelines

Mentored presentations will have 25-minute slots, where 20 minutes will
include the presentation and 5 minutes will be reserved for questions.

The number of newcomer session slots is limited, so we will probably not be
able to accept all applications.

You must submit your talk and abstract to apply for the mentoring program,
our mentors are volunteering their time and will happily provide feedback
but won't write your presentation for you!

If you are experiencing problems with Pentabarf, the proposal submission
interface, or have other questions, you can email our devroom mailing
list[2] and we will try to help you.

How to Apply

In addition to agre

Re: [Qemu-devel] [PATCH 1/3] s390x/css: unrestrict cssids

[Cornelia Huck]

On Fri,  1 Dec 2017 15:31:34 +0100
Halil Pasic  wrote:

> The default css 0xfe is currently restricted to virtual subchannel
> devices. The hope when the decision was made was, that non-virtual
> subchannel devices will come around when guests can exploit multiple
> channel subsystems. Since current guests don't do that, the pain of the
> partitioned (cssid) namespace outweighs the gain.
> 
> The default css 0xfe is currently restricted to virtual subchannel
> devices. The hope when the decision was made was, that non-virtual
> subchannel devices will come around when guest can exploit multiple
> channel subsystems. Since the guests generally don't do, the pain
> of the partitioned (cssid) namespace outweighs the gain.

Doubled paragraph?

> 
> Let us remove the corresponding restrictions (virtual devices
> can be put only in 0xfe and non-virtual devices in any css except
> the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).
> 
> At the same time, change our schema for generating css bus ids to put
> both virtual and non-virtual devices into the default css (spilling over
> into other css images, if needed). The intention is to deprecate
> s390-squash-mcss. Whit this change devices without a specified devno

s/Whit/With/

> won't end up hidden to guests not supporting multiple channel subsystems,
> unless this can not be avoided (default css full).
> 
> Deprecaton of s390-squash-mcss and indicating the changes via QMP is
> expected to follow soon (as separate commits).

Let's drop this paragraph (the qmp interface should be squashed in, and
you mention the deprecation right above.)

> 
> The adverse effect of getting rid of the restriction on migration should
> not be too severe.  Vfio-ccw devices are not live-migratable yet, and for
> virtual devices using the extra freedom would only make sense with the
> aforementioned guest support in place.
> 
> The auto-generated bus ids are affected by both changes. We hope to not
> encounter any auto-generated bus ids in production as Libvirt is always
> explicit about the bus id.  Since 8ed179c937 ("s390x/css: catch section
> mismatch on load", 2017-05-18) the worst that can happen because the same
> device ended up having a different bus id is a cleanly failed migration.
> I find it hard to reason about the impact of changed auto-generated bus
> ids on migration for command line users as I don't know which rules is
> such an user supposed to follow.

Should we document somewhere that guests supposed to be migrated should
make sure that they use explicit devnos?

> 
> Another pain-point is down- or upgrade of QEMU for command line users.
> The old way and the new way of doing vfio-ccw are mutually incompatible.
> Libvirt is only going to support the new way, so for libvirt users, the
> possible problems at QEMU downgrade are the following. If a domain
> contains virtual devices placed into a css different than 0xfe the domain
> will refuse to start with a QEMU not having this patch. Putting devices
> into a css different that 0xfe however won't make much sense in the near
> future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU
> not having this patch. This is business as usual.

My writing style would be to have this as a shorter, bulleted list -
but no need to rewrite this if this is understandable to the others on
cc:

> 
> Signed-off-by: Halil Pasic 
> 
> ---
> Hi!
> 
> I've factored out the announcing via QMP interface stuff to ease review.
> I would not mind the two being squashed together before this hits main,
> as I would much prefer having the two as one (atomic) change. But the
> second part turned out so controversial, that splitting is expected to
> benefit the review process.
> 
> v2 -> v3:
> * factored out announcing into a separate patch
> * reworded commit message
> * removed outdated comment about squash
> 
> v1 -> v2:
> * changed ccw bus id generation too (see commit message)
> * moved the property to the machine (see cover letter)
> * added a description to the property
> ---
>  hw/s390x/3270-ccw.c|  2 +-
>  hw/s390x/css.c | 28 
>  hw/s390x/s390-ccw.c|  2 +-
>  hw/s390x/s390-virtio-ccw.c |  1 -
>  hw/s390x/virtio-ccw.c  |  2 +-
>  include/hw/s390x/css.h | 12 
>  6 files changed, 11 insertions(+), 36 deletions(-)
> 

> @@ -2396,19 +2386,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool 
> is_virtual, bool squash_mcss,
> bus_id.devid, &schid, errp)) {
>  return NULL;
>  }
> -} else if (squash_mcss || is_virtual) {
> -bus_id.cssid = channel_subsys.default_cssid;
> -
> -if (!css_find_free_subch_and_devno(bus_id.cssid, &bus_id.ssid,
> -   &bus_id.devid, &schid, errp)) {
> -return NULL;
> -}
>  } else {
> -for (bus_id.cssid = 0; bus_id.cssid < MAX_CSSID; ++bus_id.cssid) {
> -   

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Cornelia Huck]

On Fri,  1 Dec 2017 15:31:35 +0100
Halil Pasic  wrote:

> Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
> to the management software (so it can tell are cssids unrestricted or
> restricted).
> 
> Signed-off-by: Halil Pasic 
> ---
> 
> Boris says having the property on the virtual-css-bridge is good form
> Libvirt PoV. @Shalini: could you verify that things work out fine
> (provided we get at least a preliminary blessing from Connie).
> 
> Consider squashing into "s390x/css: unrestrict cssids".
> ---
>  hw/s390x/css-bridge.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> index c4a9735d71..c7e8998680 100644
> --- a/hw/s390x/css-bridge.c
> +++ b/hw/s390x/css-bridge.c
> @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static bool prop_get_true(Object *obj, Error **errp)
> +{
> +return true;
> +}
> +
>  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
>  {
>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
> *klass, void *data)
>  hc->unplug = ccw_device_unplug;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  dc->props = virtual_css_bridge_properties;
> +object_class_property_add_bool(klass, "cssid-unrestricted",
> +   prop_get_true, NULL, NULL);
> +object_class_property_set_description(klass, "cssid-unrestricted",
> +"A css device can use any  cssid, regardless whether virtual"

extra space -^

> +" or not (read only, always true)",
> +NULL);
>  }
>  
>  static const TypeInfo virtual_css_bridge_info = {

Looks reasonable. If this works as expected, I'll squash it into the
previous patch.

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Christian Borntraeger]

On 12/01/2017 03:31 PM, Halil Pasic wrote:
> Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
> to the management software (so it can tell are cssids unrestricted or
> restricted).
> 
> Signed-off-by: Halil Pasic 
> ---
> 
> Boris says having the property on the virtual-css-bridge is good form
> Libvirt PoV. @Shalini: could you verify that things work out fine
> (provided we get at least a preliminary blessing from Connie).
> 
> Consider squashing into "s390x/css: unrestrict cssids".



FWIW, a property on the  virtual-css-bridge is
Acked-by: Christian Borntraeger 

So if Conny is also ok with this we might have found our solution.

> ---
>  hw/s390x/css-bridge.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> index c4a9735d71..c7e8998680 100644
> --- a/hw/s390x/css-bridge.c
> +++ b/hw/s390x/css-bridge.c
> @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
>  DEFINE_PROP_END_OF_LIST(),
>  };
> 
> +static bool prop_get_true(Object *obj, Error **errp)
> +{
> +return true;
> +}
> +
>  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
>  {
>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
> *klass, void *data)
>  hc->unplug = ccw_device_unplug;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  dc->props = virtual_css_bridge_properties;
> +object_class_property_add_bool(klass, "cssid-unrestricted",
> +   prop_get_true, NULL, NULL);
> +object_class_property_set_description(klass, "cssid-unrestricted",
> +"A css device can use any  cssid, regardless whether virtual"
> +" or not (read only, always true)",
> +NULL);
>  }
> 
>  static const TypeInfo virtual_css_bridge_info = {
> 

Re: [Qemu-devel] [qemu-s390x] [PATCH RFC 2/2] s390x: attach autogenerated nics

[Christian Borntraeger]

On 11/28/2017 02:46 PM, Cornelia Huck wrote:
> The autogenerated nics should be treated as any other device; use
> qdev_set_id() to have them show up under peripheral-anon.
> 
I think this is fine, but then I ask myself how x86 does this. So I tried to 
find out how the pc-q35 machine does this but I somehow failed to understand
how they do it. Do you have any clue?

> Signed-off-by: Cornelia Huck 
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index a23b8aec9f..830bae9d0f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -35,6 +35,7 @@
>  #include "cpu_models.h"
>  #include "qapi/qmp/qerror.h"
>  #include "hw/nmi.h"
> +#include "include/monitor/qdev.h"
> 
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> @@ -259,6 +260,7 @@ static void s390_create_virtio_net(BusState *bus, const 
> char *name)
> 
>  dev = qdev_create(bus, name);
>  qdev_set_nic_properties(dev, nd);
> +qdev_set_id(dev, NULL);
>  qdev_init_nofail(dev);
>  }
>  }
> 

Re: [Qemu-devel] [PATCH 3/3] s390x: deprecate s390-squash-mcss machine prop

[Cornelia Huck]

On Fri,  1 Dec 2017 15:31:36 +0100
Halil Pasic  wrote:

> With the cssids unrestricted (commit  "s390x/css: unrestrict

I won't add the commit id, as I intend to merge this in one go and I
don't want to edit this every time I rebase prior to pull.

> cssids") the s390-squash-mcss machine property should not be used.
> Actually libvirt never supported this, so the expectation is that
> removing it should be pretty painless.  But let's play nice and deprecate
> it first.
> 
> Signed-off-by: Halil Pasic 
> ---
> 
> I would like to reference a commit for "s390x/css: unrestrict cssids"
> which is currently in RFC status on the list.
> ---
>  hw/s390x/s390-virtio-ccw.c | 6 +-
>  qemu-doc.texi  | 8 
>  qemu-options.hx| 8 +++-
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4d65a50334..3796f666e6 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, 
> bool value,
>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>  
>  ms->s390_squash_mcss = value;
> +if (ms->s390_squash_mcss) {
> +warn_report("The machine property 's390-squash-mcss' is deprecated"
> +" (obsoleted by lifting the cssid restrictions).");

Too bad that we can't warn when this is explicitly set to false - OTOH
I don't really expect anyone doing that.

> +}
>  }
>  
>  static inline void s390_machine_initfn(Object *obj)
> @@ -582,7 +586,7 @@ static inline void s390_machine_initfn(Object *obj)
>  object_property_add_bool(obj, "s390-squash-mcss",
>   machine_get_squash_mcss,
>   machine_set_squash_mcss, NULL);
> -object_property_set_description(obj, "s390-squash-mcss",
> +object_property_set_description(obj, "s390-squash-mcss", "(deprecated) "
>  "enable/disable squashing subchannels into the default css",
>  NULL);
>  object_property_set_bool(obj, false, "s390-squash-mcss", NULL);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index db2351c746..874432d87c 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>  
>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>  
> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
> +
> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
> +cssid to be chosen freely. Instead of squashing subchannels into the
> +default channel subsystem image for guests that do not support multiple
> +channel subsystems, all devices can be put into the default channel
> +subsystem image.
> +
>  @section qemu-img command line arguments
>  
>  @subsection convert -s (since 2.0.0)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f11c4ac960..fe0c29271f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -43,7 +43,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>  "suppress-vmdesc=on|off disables self-describing 
> migration (default=off)\n"
>  "nvdimm=on|off controls NVDIMM support (default=off)\n"
>  "enforce-config-section=on|off enforce configuration 
> section migration (default=off)\n"
> -"s390-squash-mcss=on|off controls support for squashing 
> into default css (default=off)\n",
> +"s390-squash-mcss=on|off (deprecated) controls support 
> for squashing into default css (default=off)\n",
>  QEMU_ARCH_ALL)
>  STEXI
>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
> @@ -98,6 +98,12 @@ Enables or disables NVDIMM support. The default is off.
>  @item s390-squash-mcss=on|off
>  Enables or disables squashing subchannels into the default css.
>  The default is off.
> +NOTE: This property is deprecated and will be removed in future releases.
> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
> +cssid to be chosen freely. Instead of squashing subchannels into the
> +default channel subsystem image for guests that do not support multiple
> +channel subsystems, all devices can be put into the default channel
> +subsystem image.
>  @item enforce-config-section=on|off
>  If @option{enforce-config-section} is set to @var{on}, force migration
>  code to send configuration section even if the machine-type sets the

Looks sane. We should put a note into the 2.12 changelog as well.

Re: [Qemu-devel] [qemu-s390x] [PATCH RFC 0/2] s390x: cut down on unattached devices

[David Hildenbrand]

On 28.11.2017 14:46, Cornelia Huck wrote:
> info qom-tree shows several devices under unattached that probably
> should go somewhere.
> 
> The css bridge should attach to the machine, as it has a similar
> purpose as e.g. a pci host bridge.
> 
> The autogenerated network devices should be in the same bucket as any
> other device; I'm just not sure about the way I went about it.
> 
> The zpci devices are still problematic: I don't have a good idea where
> they should show up.
> 
> Remaining in the unattached container are the sysbus, memory regions
> and cpus.
> 
> Cornelia Huck (2):
>   s390x/css: attach css bridge
>   s390x: attach autogenerated nics
> 
>  hw/s390x/css-bridge.c  | 2 ++
>  hw/s390x/s390-virtio-ccw.c | 2 ++
>  2 files changed, 4 insertions(+)
> 

Reviewed-by: David Hildenbrand 

My guess is, that this should not break migration (or anything else).

-- 

Thanks,

David / dhildenb

[Qemu-devel] [Bug 1318474] Re: QEMU update causes Windows reactivation

[Yan Vugenfirer]

When updating QEMU use specific machine type and this will keep "old"
HW.

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

Title:
  QEMU update causes Windows reactivation

Status in QEMU:
  Expired

Bug description:
  After updating QEMU the guest OS's detect new hardware. As a result
  any Windows OS sees it as a significant change in hardware and require
  a reactivation.

  Host OS: Ubuntu 14.04 64-bit

  Guest OS's:
  Windows Server 2003 R2 Enterprise
  Windows Server 2008 R2 Enterprise
  Windows Server 2008 R2 Web
  Windows Server 2008 R2 Data Center

  QEMU version: 2.0.0

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

[Qemu-devel] [Bug 1736042] [NEW] qemu-system-x86_64 does not boot image reliably

[Launchpad Bug Tracker]

You have been subscribed to a public bug:

Booting image as root user with following command works randomly.

./qemu-system-x86_64 -enable-kvm -curses -smp cpus=4 -m 8192
/root/ructfe2917_g.qcow2

Most of the time it ends up on "800x600 Graphic mode"(been stuck there
even for 4 hours before killed), but 1 out of ~20 it boots image
correctly(and instantly).

This is visible in v2.5.0 build from sources, v2.5.0 from Ubuntu Xenial
and v2.1.2 from Debian Jessie.

The image in question was converted from vmdk using:

qemu-img convert -O qcow2 file.vmdk file.qcow2

The image contains Ubuntu with grub.

I can provide debug logs, but will need guidance how to enable them(and
what logs are necessary).

As a side note, it seems that booting is more certain after
connecting(or mounting) partition using qemu-nbd/mount.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
qemu-system-x86_64 does not boot image reliably
https://bugs.launchpad.net/bugs/1736042
You received this bug notification because you are a member of qemu-devel-ml, 
which is subscribed to QEMU.

Re: [Qemu-devel] [PATCH v3] rcu: reduce more than 7MB heap memory by malloc_trim()

[Yang Zhong]

On Fri, Dec 01, 2017 at 01:52:49PM +0100, Paolo Bonzini wrote:
> On 01/12/2017 11:56, Yang Zhong wrote:
> >   This issue should be caused by much times of system call by malloc_trim(),
> >   Shannon's test script include 60 scsi disks and 31 ioh3420 devices. We 
> > need 
> >   trade-off between VM perforamance and memory optimization. Whether below 
> >   method is suitable?
> > 
> >   int num=1;
> >   ..
> > 
> >   #if defined(CONFIG_MALLOC_TRIM)
> > if(!(num++%5))
> > {
> >  malloc_trim(4 * 1024 * 1024);
> > }
> >   #endif
> >  
> >   Any comments are welcome! Thanks a lot!
> 
> Indeed something like this will do, perhaps only trim once per second?
> 
  Hello Paolo,

  Thanks for comments!
  If we do trim once per second, maybe the frequency is a little high, what'e
  more, we need maintain one timer to call this, this also cost cpu resource.

  I added the log and did the test here with my test qemu command, when VM 
bootup,
  which did more than 600 times free operations and 9 times memory trim in rcu 
  thread. If i use our ClearContainer qemu command, the memory trim will down 
  to 6 times. As for Shannon's test command, the malloc trim number will 
abosultly 
  increse.

  In my above method, the trim is only executed in the multiple of 5, which will
  reduce trim times and do not heavily impact VM bootup performance. 

  I also want to use synchronize_rcu() and free() to replace call_rcu(), but 
this
  method serialize to malloc() and free(), which will reduce VM performance.

  The ultimate aim is to reduce trim system call during the VM bootup and 
running.
  It's appreciated that if you have better suggestions.

  Regards,

  Yang

> Thanks,
> 
> Paolo

Re: [Qemu-devel] [Bug 1735384] Re: OpenJDK JVM segfaults on qemu-sh4 (regression)

[John Paul Adrian Glaubitz]

On 12/04/2017 10:29 AM, Alex Bennée wrote:
> It's hard to imagine a scenario where taking the tb_lock() for resolving
> something that will fail is going to be an improvement. However maybe
> there is a subtle difference with sh4's javavm implementation.

So, OpenJDK doesn't have a SH-specific implementation of the JVM, it just
uses the Zero variant, which is a pure C++ implementation of the JVM.

The same implementation is used on any other architecture like older ARM
(< ARMv7). I just tested it on ARMv4T and it doesn't crash there on
qemu-user.

However, SH4 is special due to its implementation of atomics in user
space called gUSA for which support to qemu-user has been recently
added by Richard Hendersson. Maybe the problem lies there.

> A backtrace QEMU after the segv would be useful here.

I forgot what the proper procedure is for running qemu-user inside
GDB. Could you help me with that?

The strace looks like this in any case:

28856 access("/etc/ld.so.nohwcap",F_OK) = -1 errno=2 (No such file or directory)
28856 open("/lib/sh4-linux-gnu/libgcc_s.so.1",O_RDONLY|O_CLOEXEC) = 3
28856 read(3,0x7fffacd4,512) = 512
28856 fstat64(3,0x7fffabe8) = 0
28856 mmap(NULL,189084,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x7ee27000
28856 mprotect(0x7ee45000,61440,PROT_NONE) = 0
28856 
mmap(0x7ee54000,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x1d000)
 = 0x7ee54000
28856 close(3) = 0
28856 mprotect(0x7ee54000,4096,PROT_READ) = 0
28856 mprotect(0x7eee8000,4096,PROT_READ) = 0
28856 mprotect(0x7f05c000,20480,PROT_READ) = 0
28856 mprotect(0x7f5c8000,53248,PROT_READ) = 0
28856 getpid() = 28856
28856 munmap(0x7f065000,50134) = 0
28856 getpid() = 28856
28856 
mmap(NULL,1572864,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|0x2,-1,0) 
= 0x7eca7000
28856 mprotect(0x7eca7000,4096,PROT_NONE) = 0
28856 
clone(CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,child_stack=0x7ee26048,parent_tidptr=0x7ee26528,tls=0x7ee26930,child_tidptr=0x7ee26528)
 = 28860
28856 futex(0x7ee26528,FUTEX_WAIT,28860,NULL,0x7f77c6e8,2138556136)28856 
set_robust_list(2128766256,12,-1,2128766652,-1,2128764832) = -1 errno=38 
(Function not implemented)
--- SIGSEGV {si_signo=SIGSEGV, si_code=1, si_addr=0x289da000} ---
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault
(sid-sh4-sbuild)root@nofan:/local_scratch/sid-sh4-sbuild#

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

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

Title:
  OpenJDK JVM segfaults on qemu-sh4 (regression)

Status in QEMU:
  New

Bug description:
  Some of the recent changes introduced a regression which makes the
  OpenJDK JVM crash on qemu-sh4:

  (sid-sh4-sbuild)root@nofan:/# java -version
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault
  (sid-sh4-sbuild)root@nofan:/#

  An older version works fine:

  (sid-sh4-sbuild)root@nofan:/# java -version
  openjdk version "9.0.1"
  OpenJDK Runtime Environment (build 9.0.1+11-Debian-1)
  OpenJDK Zero VM (build 9.0.1+11-Debian-1, interpreted mode)
  (sid-sh4-sbuild)root@nofan:/#

  Haven't had time for bisecting this yet.

  Adrian

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

Re: [Qemu-devel] [PATCH v3] qemu-img: add the simplest format recognition

[Kevin Wolf]

Am 02.12.2017 um 00:49 hat Klim Kireev geschrieben:
> Now, if you type something like
> 
> qemu-img create disk.qcow2 1G
> or
> qemu-img dd if=/dev/sda of=disk.qcow2
> 
> it creates a raw image and if you need you should
> manually specify an image format with -f qcow2. It would
> be more convenient if it could be assumed from an extension.
> 
> This patch adds a simple heuristic to recognize the image format
> for qcow, qcow2, vmdk, vhdx, vdi
> 
> It warns users about guessed format and informs them about '-f' option.
> 
> Signed-off-by: Klim Kireev 

This is an incompatible change.

If we want to go there, we must introduce a deprecation warning now
without any other change in behaviour. We need to warn users for two
releases that the behaviour will change in the future, and only then we
can switch over (i.e. in qemu 2.14 the earliest if we introduce the
warning in 2.12).

I'm not sure if we even want to automatically guess the format from the
filename, or if a warning/error would be enough even in the long term.

The behaviour I have in mind is like this:

* qemu-img create x.raw 4G=> works, possibly warning
* qemu-img create -f raw x.raw 4G => works
* qemu-img create x.qcow2 4G  => error, need -f for non-raw
* qemu-img create -f qcow2 x.qcow2 4G => works
* qemu-img create -f raw x.qcow2 4G   => works, possibly warning

Kevin

Re: [Qemu-devel] [PATCH v3] rcu: reduce more than 7MB heap memory by malloc_trim()

[Daniel P. Berrange]

On Mon, Dec 04, 2017 at 08:03:22PM +0800, Yang Zhong wrote:
> On Fri, Dec 01, 2017 at 01:52:49PM +0100, Paolo Bonzini wrote:
> > On 01/12/2017 11:56, Yang Zhong wrote:
> > >   This issue should be caused by much times of system call by 
> > > malloc_trim(),
> > >   Shannon's test script include 60 scsi disks and 31 ioh3420 devices. We 
> > > need 
> > >   trade-off between VM perforamance and memory optimization. Whether 
> > > below 
> > >   method is suitable?
> > > 
> > >   int num=1;
> > >   ..
> > > 
> > >   #if defined(CONFIG_MALLOC_TRIM)
> > > if(!(num++%5))
> > > {
> > >  malloc_trim(4 * 1024 * 1024);
> > > }
> > >   #endif
> > >  
> > >   Any comments are welcome! Thanks a lot!
> > 
> > Indeed something like this will do, perhaps only trim once per second?
> > 
>   Hello Paolo,
> 
>   Thanks for comments!
>   If we do trim once per second, maybe the frequency is a little high, what'e
>   more, we need maintain one timer to call this, this also cost cpu resource.
> 
>   I added the log and did the test here with my test qemu command, when VM 
> bootup,
>   which did more than 600 times free operations and 9 times memory trim in 
> rcu 
>   thread. If i use our ClearContainer qemu command, the memory trim will down 
>   to 6 times. As for Shannon's test command, the malloc trim number will 
> abosultly 
>   increse.
> 
>   In my above method, the trim is only executed in the multiple of 5, which 
> will
>   reduce trim times and do not heavily impact VM bootup performance. 
> 
>   I also want to use synchronize_rcu() and free() to replace call_rcu(), but 
> this
>   method serialize to malloc() and free(), which will reduce VM performance.
> 
>   The ultimate aim is to reduce trim system call during the VM bootup and 
> running.
>   It's appreciated that if you have better suggestions.

Does configuring QEMU to use tcmalloc or jemalloc instead of glibc's malloc
give you the performance & menmory usage that you require? If so, it might
not be worth bothering to hack around problems in glibc's malloc at all.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [Bug 1736042] Re: qemu-system-x86_64 does not boot image reliably

[tezeb]

I want a fix ;) and I do expect qemu to get it faster then Ubuntu.
So I set up Ubuntu Zesty(to fullfill dependencies) and build qemu:
QEMU emulator version 2.10.1 (v2.10.1-dirty)
and... it's still there.

Can you tell me how and what logs to provide?

** Package changed: qemu (Ubuntu) => qemu

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

Title:
  qemu-system-x86_64 does not boot image reliably

Status in QEMU:
  New

Bug description:
  Booting image as root user with following command works randomly.

  ./qemu-system-x86_64 -enable-kvm -curses -smp cpus=4 -m 8192
  /root/ructfe2917_g.qcow2

  Most of the time it ends up on "800x600 Graphic mode"(been stuck there
  even for 4 hours before killed), but 1 out of ~20 it boots image
  correctly(and instantly).

  This is visible in v2.5.0 build from sources, v2.5.0 from Ubuntu
  Xenial and v2.1.2 from Debian Jessie.

  The image in question was converted from vmdk using:

  qemu-img convert -O qcow2 file.vmdk file.qcow2

  The image contains Ubuntu with grub.

  I can provide debug logs, but will need guidance how to enable
  them(and what logs are necessary).

  As a side note, it seems that booting is more certain after
  connecting(or mounting) partition using qemu-nbd/mount.

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

[Qemu-devel] [Bug 1736042] Re: qemu-system-x86_64 does not boot image reliably

[tezeb]

And it still happens on:

QEMU emulator version 2.10.93 (v2.11.0-rc3-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

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

Title:
  qemu-system-x86_64 does not boot image reliably

Status in QEMU:
  New

Bug description:
  Booting image as root user with following command works randomly.

  ./qemu-system-x86_64 -enable-kvm -curses -smp cpus=4 -m 8192
  /root/ructfe2917_g.qcow2

  Most of the time it ends up on "800x600 Graphic mode"(been stuck there
  even for 4 hours before killed), but 1 out of ~20 it boots image
  correctly(and instantly).

  This is visible in v2.5.0 build from sources, v2.5.0 from Ubuntu
  Xenial and v2.1.2 from Debian Jessie.

  The image in question was converted from vmdk using:

  qemu-img convert -O qcow2 file.vmdk file.qcow2

  The image contains Ubuntu with grub.

  I can provide debug logs, but will need guidance how to enable
  them(and what logs are necessary).

  As a side note, it seems that booting is more certain after
  connecting(or mounting) partition using qemu-nbd/mount.

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

Re: [Qemu-devel] [PATCH v13 00/12] Add ARMv8 RAS virtualization support in QEMU

[gengdongjiu]

ping again.

thanks


On 2017/11/28 2:39, Dongjiu Geng wrote:
> From: gengdongjiu 
> 
> In the ARMv8 platform, the CPU error type are synchronous external
> abort(SEA) and SError Interrupt (SEI). If guest happen exception, 
> sometimes  guest itself do the recovery is better, because host 
> does not know guest's detailed info. For example, if a guest
> user-space application happen exception, guest can kill this 
> application, but host can not do that.
> 
> For the ARMv8 SEA/SEI, KVM or host kernel will deliver SIGBUS or
> use other interface to notify user space. After user space gets 
> the notification, it will record the CPER to guest GHES buffer
> for guest and inject a exception or IRQ to KVM.
> 
> In the current implement, if the SIGBUS is BUS_MCEERR_AR, we will
> treat it as synchronous exception, and use ARMv8 SEA notification type
> to notify guest after recording CPER for guest; If the SIGBUS is
> BUS_MCEERR_AO, we will treat it as asynchronous exception, and use
> GPIO-Signal to notify guest after recording CPER for guest.
> 
> If KVM wants userspace to do the recovery for the SError, it will return a 
> error
> status to Qemu. Then Qemu will specify the guest ESR value and inject a 
> virtual
> SError.
> 
> This series patches have three parts:
> 1. Generate APEI/GHES table and record CPER for guest in runtime.
> 2. Handle the SIGBUS signal, record the CPER and fill into guest memory,
>then according to SIGBUS type(BUS_MCEERR_AR or BUS_MCEERR_AO), using
>different ACPI notification type to notify guest.
> 3. Specify guest SError ESR value and inject a virtual SError 
> 
> Whole solution was suggested by James(james.mo...@arm.com); inject RAS SEA 
> abort and specify guest ESR
> in user space are suggested by Marc(marc.zyng...@arm.com), APEI part solution 
> is suggested by
> Laszlo(ler...@redhat.com). Shown some discussion in [1].
> 
> 
> This series patches have already tested on ARM64 platform with RAS feature 
> enabled:
> Show the APEI part verification result in [2]
> Show the BUS_MCEERR_AR and BUS_MCEERR_AO SIGBUS handling verification result 
> in [3]
> Show Qemu set guest ESR and inject virtual SError verification result in [4]
> 
> ---
> Change since v12:
> 1. Address Paolo's comments to move HWPoisonPage definition to 
> accel/kvm/kvm-all.c
> 2. Only call kvm_cpu_synchronize_state() when get the BUS_MCEERR_AR signal
> 3. Only add and enable GPIO-Signal and ARMv8 SEA two hardware error sources
> 4. Address Michael's comments to not sync SPDX from Linux kernel header file 
> 
> Change since v11:
> Address James's comments(james.mo...@arm.com)
> 1. Check whether KVM has the capability to to set ESR instead of detecting 
> host CPU RAS capability
> 2. For SIGBUS_MCEERR_AR SIGBUS, use Synchronous-External-Abort(SEA) 
> notification type
>for SIGBUS_MCEERR_AO SIGBUS, use GPIO-Signal notification
> 
> 
> Address Shannon's comments(for ACPI part):
> 1. Unify hest_ghes.c and hest_ghes.h license declaration
> 2. Remove unnecessary including "qmp-commands.h" in hest_ghes.c
> 3. Unconditionally add guest APEI table based on James's 
> comments(james.mo...@arm.com) 
> 4. Add a option to virt machine for migration compatibility. On new virt 
> machine it's on
>by default while off for old ones, we enabled it since 2.10
> 5. Refer to the ACPI spec version which introduces Hardware Error 
> Notification first time
> 6. Add ACPI_HEST_NOTIFY_RESERVED notification type
> 
> Address Igor's comments(for ACPI part):
> 1. Add doc patch first which will describe how it's supposed to work between 
> QEMU/firmware/guest
>OS with expected flows.
> 2. Move APEI diagrams into doc/spec patch
> 3. Remove redundant g_malloc in ghes_record_cper()
> 4. Use build_append_int_noprefix() API to compose whole error status block 
> and whole APEI table, 
>and try to get rid of most structures in patch 1, as they will be left 
> unused after that
> 5. Reuse something like 
> https://github.com/imammedo/qemu/commit/3d2fd6d13a3ea298d2ee814835495ce6241d085c
>to build GAS
> 6. Remove much offsetof() in the function
> 7. Build independent tables first and only then build dependent tables 
> passing to it pointers
>to previously build table if necessary.
> 8. Redefine macro GHES_ACPI_HEST_NOTIFY_RESERVED to 
> ACPI_HEST_ERROR_SOURCE_COUNT to avoid confusion
> 
> 
> Address Peter Maydell's comments
> 1. linux-headers is done as a patch of their own created using 
> scripts/update-linux-headers.sh run against a
>mainline kernel tree 
> 2. Tested whether this patchset builds OK on aarch32  
> 3. Abstract Hwpoison page adding code  out properly into a cpu-independent 
> source file from target/i386/kvm.c,
>such as kvm-all.c
> 4. Add doc-comment formatted documentation comment for new globally-visible 
> function prototype in a header
> 
> ---
> [1]:
> https://lkml.org/lkml/2017/2/27/246
> https://patchwork.kernel.org/patch/9633105/
> https://patchwork.kernel.org/patch/9925227/
> 
> [2]:
> Not

Re: [Qemu-devel] [PATCH v3] rcu: reduce more than 7MB heap memory by malloc_trim()

[Yang Zhong]

On Mon, Dec 04, 2017 at 12:07:05PM +, Daniel P. Berrange wrote:
> On Mon, Dec 04, 2017 at 08:03:22PM +0800, Yang Zhong wrote:
> > On Fri, Dec 01, 2017 at 01:52:49PM +0100, Paolo Bonzini wrote:
> > > On 01/12/2017 11:56, Yang Zhong wrote:
> > > >   This issue should be caused by much times of system call by 
> > > > malloc_trim(),
> > > >   Shannon's test script include 60 scsi disks and 31 ioh3420 devices. 
> > > > We need 
> > > >   trade-off between VM perforamance and memory optimization. Whether 
> > > > below 
> > > >   method is suitable?
> > > > 
> > > >   int num=1;
> > > >   ..
> > > > 
> > > >   #if defined(CONFIG_MALLOC_TRIM)
> > > > if(!(num++%5))
> > > > {
> > > >  malloc_trim(4 * 1024 * 1024);
> > > > }
> > > >   #endif
> > > >  
> > > >   Any comments are welcome! Thanks a lot!
> > > 
> > > Indeed something like this will do, perhaps only trim once per second?
> > > 
> >   Hello Paolo,
> > 
> >   Thanks for comments!
> >   If we do trim once per second, maybe the frequency is a little high, 
> > what'e
> >   more, we need maintain one timer to call this, this also cost cpu 
> > resource.
> > 
> >   I added the log and did the test here with my test qemu command, when VM 
> > bootup,
> >   which did more than 600 times free operations and 9 times memory trim in 
> > rcu 
> >   thread. If i use our ClearContainer qemu command, the memory trim will 
> > down 
> >   to 6 times. As for Shannon's test command, the malloc trim number will 
> > abosultly 
> >   increse.
> > 
> >   In my above method, the trim is only executed in the multiple of 5, which 
> > will
> >   reduce trim times and do not heavily impact VM bootup performance. 
> > 
> >   I also want to use synchronize_rcu() and free() to replace call_rcu(), 
> > but this
> >   method serialize to malloc() and free(), which will reduce VM performance.
> > 
> >   The ultimate aim is to reduce trim system call during the VM bootup and 
> > running.
> >   It's appreciated that if you have better suggestions.
> 
> Does configuring QEMU to use tcmalloc or jemalloc instead of glibc's malloc
> give you the performance & menmory usage that you require? If so, it might
> not be worth bothering to hack around problems in glibc's malloc at all.
> 
> 
  Hello Daniel,

  Thanks for comment!
  I used the tcmalloc and jemalloc to do compared test, it's pity that there is 
no heap
  item in smaps file with jemalloc, the glibc and tcmalloc result as below:

  ##glibc malloc
  5618c0a98000-5618c1cde000 rw-p  00:00 0  
[heap]
  Size:  18712 kB
  KernelPageSize:4 kB
  MMUPageSize:   4 kB
  Rss:   10536 kB
  Pss:   10536 kB
 
  ##tcmalloc
  557f79119000-557f7af46000 rw-p  00:00 0  
[heap]
  Size:  30900 kB
  Rss:   20244 kB
  Pss:   20244 kB

  The result show the Rss with tcmalloc is higher than glibc's.

  Regards,

  Yang

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3] rcu: reduce more than 7MB heap memory by malloc_trim()

[Paolo Bonzini]

On 04/12/2017 13:07, Daniel P. Berrange wrote:
> On Mon, Dec 04, 2017 at 08:03:22PM +0800, Yang Zhong wrote:
>> On Fri, Dec 01, 2017 at 01:52:49PM +0100, Paolo Bonzini wrote:
>>> On 01/12/2017 11:56, Yang Zhong wrote:
   This issue should be caused by much times of system call by 
 malloc_trim(),
   Shannon's test script include 60 scsi disks and 31 ioh3420 devices. We 
 need 
   trade-off between VM perforamance and memory optimization. Whether below 
   method is suitable?

   int num=1;
   ..

   #if defined(CONFIG_MALLOC_TRIM)
 if(!(num++%5))
 {
  malloc_trim(4 * 1024 * 1024);
 }
   #endif
  
   Any comments are welcome! Thanks a lot!
>>>
>>> Indeed something like this will do, perhaps only trim once per second?
>>>
>>   Hello Paolo,
>>
>>   Thanks for comments!
>>   If we do trim once per second, maybe the frequency is a little high, what'e
>>   more, we need maintain one timer to call this, this also cost cpu resource.
>>
>>   I added the log and did the test here with my test qemu command, when VM 
>> bootup,
>>   which did more than 600 times free operations and 9 times memory trim in 
>> rcu 
>>   thread. If i use our ClearContainer qemu command, the memory trim will 
>> down 
>>   to 6 times. As for Shannon's test command, the malloc trim number will 
>> abosultly 
>>   increse.
>>
>>   In my above method, the trim is only executed in the multiple of 5, which 
>> will
>>   reduce trim times and do not heavily impact VM bootup performance. 
>>
>>   I also want to use synchronize_rcu() and free() to replace call_rcu(), but 
>> this
>>   method serialize to malloc() and free(), which will reduce VM performance.
>>
>>   The ultimate aim is to reduce trim system call during the VM bootup and 
>> running.
>>   It's appreciated that if you have better suggestions.
> 
> Does configuring QEMU to use tcmalloc or jemalloc instead of glibc's malloc
> give you the performance & menmory usage that you require? If so, it might
> not be worth bothering to hack around problems in glibc's malloc at all.

At least for tcmalloc, the default tuning is to pay little attention to
memory usage.

Paolo

Re: [Qemu-devel] [PATCH v3] rcu: reduce more than 7MB heap memory by malloc_trim()

[Shannon Zhao]

Hi Yang,

On 2017/12/4 20:03, Yang Zhong wrote:
> On Fri, Dec 01, 2017 at 01:52:49PM +0100, Paolo Bonzini wrote:
>> > On 01/12/2017 11:56, Yang Zhong wrote:
>>> > >   This issue should be caused by much times of system call by 
>>> > > malloc_trim(),
>>> > >   Shannon's test script include 60 scsi disks and 31 ioh3420 devices. 
>>> > > We need 
>>> > >   trade-off between VM perforamance and memory optimization. Whether 
>>> > > below 
>>> > >   method is suitable?
>>> > > 
>>> > >   int num=1;
>>> > >   ..
>>> > > 
>>> > >   #if defined(CONFIG_MALLOC_TRIM)
>>> > > if(!(num++%5))
>>> > > {
>>> > >  malloc_trim(4 * 1024 * 1024);
>>> > > }
>>> > >   #endif
>>> > >  
>>> > >   Any comments are welcome! Thanks a lot!
>> > 
>> > Indeed something like this will do, perhaps only trim once per second?
>> > 
>   Hello Paolo,
> 
>   Thanks for comments!
>   If we do trim once per second, maybe the frequency is a little high, what'e
>   more, we need maintain one timer to call this, this also cost cpu resource.
> 
>   I added the log and did the test here with my test qemu command, when VM 
> bootup,
>   which did more than 600 times free operations and 9 times memory trim in 
> rcu 
>   thread. If i use our ClearContainer qemu command, the memory trim will down 
>   to 6 times. As for Shannon's test command, the malloc trim number will 
> abosultly 
>   increse.
> 
>   In my above method, the trim is only executed in the multiple of 5, which 
> will
>   reduce trim times and do not heavily impact VM bootup performance. 
> 
>   I also want to use synchronize_rcu() and free() to replace call_rcu(), but 
> this
>   method serialize to malloc() and free(), which will reduce VM performance.
> 
>   The ultimate aim is to reduce trim system call during the VM bootup and 
> running.
>   It's appreciated that if you have better suggestions.

Maybe we can provide a QMP command or something else for user to trim
the heap manually like the kernel sysfs interface
/proc/sys/vm/drop_caches which provides an interface for user to drop
the caches.
So let user to decide whether it needs to trim the heap.

Thanks,
-- 
Shannon

Re: [Qemu-devel] [PATCH 2/2] qmp: add nbd-server-remove

[Kevin Wolf]

Am 09.11.2017 um 16:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add command for export removing. It is needed for cases when we
> don't want to keep export after the operation on it was completed.
> The other example is temporary node, created with blockdev-add.
> If we want to delete it we should firstly remove corresponding
> NBD export.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  qapi/block.json | 20 
>  blockdev-nbd.c  | 27 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/qapi/block.json b/qapi/block.json
> index f093fa3f27..1827940717 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -223,6 +223,26 @@
>  { 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 
> 'bool'} }
>  
>  ##
> +# @nbd-server-remove:
> +#
> +# Stop exporting block node through QEMU's embedded NBD server.
> +#
> +# @device: The device name or node name of the exported node. Should be equal
> +#  to @device parameter for corresponding nbd-server-add command 
> call.
> +#
> +# @force: Whether active connections to the export should be closed. If this
> +# parameter is false the export is only removed from named exports 
> list,
> +# so new connetions are impossible and it would be freed after all
> +# clients are disconnected (default false).
> +#
> +# Returns: error if the server is not running or the device is not marked for
> +#  export.
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'nbd-server-remove', 'data': {'device': 'str', '*force': 
> 'bool'} }
> +
> +##
>  # @nbd-server-stop:
>  #
>  # Stop QEMU's embedded NBD server, and unregister all devices previously
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 28f551a7b0..5f66951c33 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -203,6 +203,33 @@ void qmp_nbd_server_add(const char *device, bool 
> has_writable, bool writable,
>  nbd_export_put(exp);
>  }
>  
> +void qmp_nbd_server_remove(const char *device, bool has_force, bool force,
> +   Error **errp)
> +{
> +NBDExport *exp;
> +
> +if (!nbd_server) {
> +error_setg(errp, "NBD server not running");
> +return;
> +}
> +
> +exp = nbd_export_find(device);
> +if (exp == NULL) {
> +error_setg(errp, "'%s' is not exported", device);
> +return;
> +}
> +
> +if (!has_force) {
> +force = false;
> +}
> +
> +if (force) {
> +nbd_export_close(exp);
> +} else {
> +nbd_export_set_name(exp, NULL);
> +}
> +}

Using nbd_export_find() in order to identify the export, and
nbd_export_set_name(NULL) for force=false means that you can't follow up
with another nbd-server-remove,force=true later if the client just
doesn't want to go away voluntarily.

To be honest, I'm not very happy with the whole NBD interface, even as
it exists today. We're mixing up several things that should really be
kept separate: The export name (this should be configurable and the real
ID of the export that you use with things like nbd-server-remove), the
device/node name (just a pointer to the root node, not to be exposed or
even just stored anywhere) and whether the export is visible for clients.

nbd-server-remove with force=false should make the export invisible for
clients, but still keep the ID assigned so that another
nbd-server-remove with force=true can still be issued. We should also
really have a query-nbd-server that shows all exports with their IDs,
their root node (which may have changed meanwhile - snapshots, block
jobs, filter drivers) and information about the currently connected
client(s).

Maybe it would be worth to clean up the interfaces this way before we
add the first command that actually has to identify an export. The
existing commands should be easy to extend in a backwards compatible
way.

Kevin

Re: [Qemu-devel] [PATCH 2/2] qmp: add nbd-server-remove

[Vladimir Sementsov-Ogievskiy]

04.12.2017 15:32, Kevin Wolf wrote:

Am 09.11.2017 um 16:40 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add command for export removing. It is needed for cases when we
don't want to keep export after the operation on it was completed.
The other example is temporary node, created with blockdev-add.
If we want to delete it we should firstly remove corresponding
NBD export.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block.json | 20 
  blockdev-nbd.c  | 27 +++
  2 files changed, 47 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index f093fa3f27..1827940717 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -223,6 +223,26 @@
  { 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} 
}
  
  ##

+# @nbd-server-remove:
+#
+# Stop exporting block node through QEMU's embedded NBD server.
+#
+# @device: The device name or node name of the exported node. Should be equal
+#  to @device parameter for corresponding nbd-server-add command call.
+#
+# @force: Whether active connections to the export should be closed. If this
+# parameter is false the export is only removed from named exports 
list,
+# so new connetions are impossible and it would be freed after all
+# clients are disconnected (default false).
+#
+# Returns: error if the server is not running or the device is not marked for
+#  export.
+#
+# Since: 2.12
+##
+{ 'command': 'nbd-server-remove', 'data': {'device': 'str', '*force': 'bool'} }
+
+##
  # @nbd-server-stop:
  #
  # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 28f551a7b0..5f66951c33 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -203,6 +203,33 @@ void qmp_nbd_server_add(const char *device, bool 
has_writable, bool writable,
  nbd_export_put(exp);
  }
  
+void qmp_nbd_server_remove(const char *device, bool has_force, bool force,

+   Error **errp)
+{
+NBDExport *exp;
+
+if (!nbd_server) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
+exp = nbd_export_find(device);
+if (exp == NULL) {
+error_setg(errp, "'%s' is not exported", device);
+return;
+}
+
+if (!has_force) {
+force = false;
+}
+
+if (force) {
+nbd_export_close(exp);
+} else {
+nbd_export_set_name(exp, NULL);
+}
+}

Using nbd_export_find() in order to identify the export, and
nbd_export_set_name(NULL) for force=false means that you can't follow up
with another nbd-server-remove,force=true later if the client just
doesn't want to go away voluntarily.


reasonable, I'll think about it.



To be honest, I'm not very happy with the whole NBD interface, even as
it exists today. We're mixing up several things that should really be
kept separate: The export name (this should be configurable and the real
ID of the export that you use with things like nbd-server-remove), the
device/node name (just a pointer to the root node, not to be exposed or
even just stored anywhere) and whether the export is visible for clients.


yes. we have thought about this too, v3 will handle this problem.



nbd-server-remove with force=false should make the export invisible for
clients, but still keep the ID assigned so that another
nbd-server-remove with force=true can still be issued. We should also
really have a query-nbd-server that shows all exports with their IDs,
their root node (which may have changed meanwhile - snapshots, block
jobs, filter drivers) and information about the currently connected
client(s).

Maybe it would be worth to clean up the interfaces this way before we
add the first command that actually has to identify an export. The
existing commands should be easy to extend in a backwards compatible
way.


yes, I've already started to do it.



Kevin



--
Best regards,
Vladimir

[Qemu-devel] [PATCH v1 for-2.12 2/5] s390x/tcg: fix and cleanup mcck injection

[David Hildenbrand]

The architecture mode indication wasn't stored. The split of certain
64bit fields was unnecessary. Also, the complete clock comparator, not
just bit 0-55 (starting at byte 1) was stored.

We now generate a proper MCIC via the same helper we use for KVM.

While at it, also get rid of two local variables. There is be more to
clean up, but we will change the other parts later on either way.

Signed-off-by: David Hildenbrand 
---
 target/s390x/excp_helper.c | 18 --
 target/s390x/internal.h|  6 +++---
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index d831537544..840cf7641a 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -369,7 +369,6 @@ static void do_io_interrupt(CPUS390XState *env)
 static void do_mchk_interrupt(CPUS390XState *env)
 {
 S390CPU *cpu = s390_env_get_cpu(env);
-uint64_t mask, addr;
 LowCore *lowcore;
 MchkQueue *q;
 int i;
@@ -395,6 +394,9 @@ static void do_mchk_interrupt(CPUS390XState *env)
 
 lowcore = cpu_map_lowcore(env);
 
+/* we are always in z/Architecture mode */
+lowcore->ar_access_id = 1;
+
 for (i = 0; i < 16; i++) {
 lowcore->floating_pt_save_area[i] = cpu_to_be64(get_freg(env, i)->ll);
 lowcore->gpregs_save_area[i] = cpu_to_be64(env->regs[i]);
@@ -404,17 +406,12 @@ static void do_mchk_interrupt(CPUS390XState *env)
 lowcore->prefixreg_save_area = cpu_to_be32(env->psa);
 lowcore->fpt_creg_save_area = cpu_to_be32(env->fpc);
 lowcore->tod_progreg_save_area = cpu_to_be32(env->todpr);
-lowcore->cpu_timer_save_area[0] = cpu_to_be32(env->cputm >> 32);
-lowcore->cpu_timer_save_area[1] = cpu_to_be32((uint32_t)env->cputm);
-lowcore->clock_comp_save_area[0] = cpu_to_be32(env->ckc >> 32);
-lowcore->clock_comp_save_area[1] = cpu_to_be32((uint32_t)env->ckc);
+lowcore->cpu_timer_save_area = cpu_to_be64(env->cputm);
+lowcore->clock_comp_save_area = cpu_to_be64(env->ckc >> 8);
 
-lowcore->mcck_interruption_code[0] = cpu_to_be32(0x00400f1d);
-lowcore->mcck_interruption_code[1] = cpu_to_be32(0x4033);
+lowcore->mcic = cpu_to_be64(s390_build_validity_mcic() | MCIC_SC_CP);
 lowcore->mcck_old_psw.mask = cpu_to_be64(get_psw_mask(env));
 lowcore->mcck_old_psw.addr = cpu_to_be64(env->psw.addr);
-mask = be64_to_cpu(lowcore->mcck_new_psw.mask);
-addr = be64_to_cpu(lowcore->mcck_new_psw.addr);
 
 cpu_unmap_lowcore(lowcore);
 
@@ -426,7 +423,8 @@ static void do_mchk_interrupt(CPUS390XState *env)
 DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__,
 env->psw.mask, env->psw.addr);
 
-load_psw(env, mask, addr);
+load_psw(env, be64_to_cpu(lowcore->mcck_new_psw.mask),
+ be64_to_cpu(lowcore->mcck_new_psw.addr));
 }
 
 void s390_cpu_do_interrupt(CPUState *cs)
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 6817b2c432..1a88e4beb4 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -43,7 +43,7 @@ typedef struct LowCore {
 uint8_t pad3[0xc8 - 0xc4];/* 0x0c4 */
 uint32_tstfl_fac_list;/* 0x0c8 */
 uint8_t pad4[0xe8 - 0xcc];/* 0x0cc */
-uint32_tmcck_interruption_code[2]; /* 0x0e8 */
+uint64_tmcic; /* 0x0e8 */
 uint8_t pad5[0xf4 - 0xf0];/* 0x0f0 */
 uint32_texternal_damage_code; /* 0x0f4 */
 uint64_tfailing_storage_address;  /* 0x0f8 */
@@ -118,8 +118,8 @@ typedef struct LowCore {
 uint32_tfpt_creg_save_area;/* 0x131c */
 uint8_t pad16[0x1324 - 0x1320];/* 0x1320 */
 uint32_ttod_progreg_save_area; /* 0x1324 */
-uint32_tcpu_timer_save_area[2];/* 0x1328 */
-uint32_tclock_comp_save_area[2];   /* 0x1330 */
+uint64_tcpu_timer_save_area;   /* 0x1328 */
+uint64_tclock_comp_save_area;  /* 0x1330 */
 uint8_t pad17[0x1340 - 0x1338];/* 0x1338 */
 uint32_taccess_regs_save_area[16]; /* 0x1340 */
 uint64_tcregs_save_area[16];   /* 0x1380 */
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 4/5] s390x/tcg: indicate value of TODPR in STCKE

[David Hildenbrand]

We were not yet using the value of the TOD Programmable Register.

Signed-off-by: David Hildenbrand 
---
 target/s390x/translate.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 48b031894a..8da8610839 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3897,7 +3897,10 @@ static ExitStatus op_stcke(DisasContext *s, DisasOps *o)
 {
 TCGv_i64 c1 = tcg_temp_new_i64();
 TCGv_i64 c2 = tcg_temp_new_i64();
+TCGv_i64 todpr = tcg_temp_new_i64();
 gen_helper_stck(c1, cpu_env);
+/* 16 bit value store in an uint32_t (only valid bits set) */
+tcg_gen_ld32u_i64(todpr, cpu_env, offsetof(CPUS390XState, todpr));
 /* Shift the 64-bit value into its place as a zero-extended
104-bit value.  Note that "bit positions 64-103 are always
non-zero so that they compare differently to STCK"; we set
@@ -3905,11 +3908,13 @@ static ExitStatus op_stcke(DisasContext *s, DisasOps *o)
 tcg_gen_shli_i64(c2, c1, 56);
 tcg_gen_shri_i64(c1, c1, 8);
 tcg_gen_ori_i64(c2, c2, 0x1);
+tcg_gen_or_i64(c2, c2, todpr);
 tcg_gen_qemu_st64(c1, o->in2, get_mem_index(s));
 tcg_gen_addi_i64(o->in2, o->in2, 8);
 tcg_gen_qemu_st64(c2, o->in2, get_mem_index(s));
 tcg_temp_free_i64(c1);
 tcg_temp_free_i64(c2);
+tcg_temp_free_i64(todpr);
 /* ??? We don't implement clock states.  */
 gen_op_movi_cc(s, 0);
 return NO_EXIT;
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 5/5] s390x/tcg: wire up STORE CHANNEL REPORT WORD

[David Hildenbrand]

We somehow missed that, new kernels require it.

Signed-off-by: David Hildenbrand 
---
 target/s390x/helper.h  | 1 +
 target/s390x/insn-data.def | 1 +
 target/s390x/misc_helper.c | 9 +
 target/s390x/translate.c   | 8 
 4 files changed, 19 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 0281c286b8..2ce57edc14 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -166,6 +166,7 @@ DEF_HELPER_3(msch, void, env, i64, i64)
 DEF_HELPER_2(rchp, void, env, i64)
 DEF_HELPER_2(rsch, void, env, i64)
 DEF_HELPER_3(ssch, void, env, i64, i64)
+DEF_HELPER_2(stcrw, void, env, i64)
 DEF_HELPER_3(stsch, void, env, i64, i64)
 DEF_HELPER_3(tsch, void, env, i64, i64)
 DEF_HELPER_2(chsc, void, env, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 8c2541f545..43ab1963c8 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1055,6 +1055,7 @@
 C(0xb23b, RCHP,S, Z,   0, 0, 0, 0, rchp, 0)
 C(0xb238, RSCH,S, Z,   0, 0, 0, 0, rsch, 0)
 C(0xb233, SSCH,S, Z,   0, insn, 0, 0, ssch, 0)
+C(0xb239, STCRW,   S, Z,   0, insn, 0, 0, stcrw, 0)
 C(0xb234, STSCH,   S, Z,   0, insn, 0, 0, stsch, 0)
 C(0xb235, TSCH,S, Z,   0, insn, 0, 0, tsch, 0)
 /* ??? Not listed in PoO ninth edition, but there's a linux driver that
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 2c6ab329fb..55e78c56d2 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -385,6 +385,15 @@ void HELPER(ssch)(CPUS390XState *env, uint64_t r1, 
uint64_t inst)
 qemu_mutex_unlock_iothread();
 }
 
+void HELPER(stcrw)(CPUS390XState *env, uint64_t inst)
+{
+S390CPU *cpu = s390_env_get_cpu(env);
+
+qemu_mutex_lock_iothread();
+ioinst_handle_stcrw(cpu, inst >> 16, GETPC());
+qemu_mutex_unlock_iothread();
+}
+
 void HELPER(stsch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
 S390CPU *cpu = s390_env_get_cpu(env);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 8da8610839..5e051fdd03 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4071,6 +4071,14 @@ static ExitStatus op_stsch(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_stcrw(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+gen_helper_stcrw(cpu_env, o->in2);
+set_cc_static(s);
+return NO_EXIT;
+}
+
 static ExitStatus op_tsch(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 0/5] s390x/tcg: CCW hotplug support

[David Hildenbrand]

Hotplugging a ccw device currently fails due to different reasons.

1. The stored machine check information is partially wrong.
2. The TOD programmable field cannot be restored.
3. STCRW cannot get executed.

With these patches, I am able to sucessfully hotplug e.g. virtio-rng by
issuing "device_add virtio-rng-ccw,id=rng0" to a Linux guest.

David Hildenbrand (5):
  s390x/kvm: factor out build_channel_report_mcic() into cpu.h
  s390x/tcg: fix and cleanup mcck injection
  s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD
  s390x/tcg: indicate value of TODPR in STCKE
  s390x/tcg: wire up STORE CHANNEL REPORT WORD

 target/s390x/cpu.h | 20 
 target/s390x/excp_helper.c | 18 --
 target/s390x/helper.h  |  2 ++
 target/s390x/insn-data.def |  3 +++
 target/s390x/internal.h|  6 +++---
 target/s390x/kvm.c | 25 ++---
 target/s390x/misc_helper.c | 20 
 target/s390x/translate.c   | 20 
 8 files changed, 78 insertions(+), 36 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 1/5] s390x/kvm: factor out build_channel_report_mcic() into cpu.h

[David Hildenbrand]

We'll need it later on in two places. Refactor it to just indicate the
valid bit. While at it, introduce a define for the used CR14 bit (we'll
also need later on).

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h | 20 
 target/s390x/kvm.c | 25 ++---
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9cfbbbac04..4d3e7920f5 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -351,6 +351,9 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define CR0_CPU_TIMER_SC0x0400ULL
 #define CR0_SERVICE_SC  0x0200ULL
 
+/* Control register 14 bits */
+#define CR14_CHANNEL_REPORT_SC  0x1000ULL
+
 /* MMU */
 #define MMU_PRIMARY_IDX 0
 #define MMU_SECONDARY_IDX   1
@@ -674,6 +677,23 @@ struct sysib_322 {
 #define MCIC_VB_CT 0x0002ULL
 #define MCIC_VB_CC 0x0001ULL
 
+static inline uint64_t s390_build_validity_mcic(void)
+{
+uint64_t mcic;
+
+/* indicate all valid bits (no damage) */
+mcic = MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
+   MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
+   MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
+if (s390_has_feat(S390_FEAT_VECTOR)) {
+mcic |= MCIC_VB_VR;
+}
+if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
+mcic |= MCIC_VB_GS;
+}
+return mcic;
+}
+
 
 /* cpu.c */
 int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 97c45d5537..9b8b59f2a2 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1852,33 +1852,12 @@ void kvm_s390_io_interrupt(uint16_t subchannel_id,
 kvm_s390_floating_interrupt(&irq);
 }
 
-static uint64_t build_channel_report_mcic(void)
-{
-uint64_t mcic;
-
-/* subclass: indicate channel report pending */
-mcic = MCIC_SC_CP |
-/* subclass modifiers: none */
-/* storage errors: none */
-/* validity bits: no damage */
-MCIC_VB_WP | MCIC_VB_MS | MCIC_VB_PM | MCIC_VB_IA | MCIC_VB_FP |
-MCIC_VB_GR | MCIC_VB_CR | MCIC_VB_ST | MCIC_VB_AR | MCIC_VB_PR |
-MCIC_VB_FC | MCIC_VB_CT | MCIC_VB_CC;
-if (s390_has_feat(S390_FEAT_VECTOR)) {
-mcic |= MCIC_VB_VR;
-}
-if (s390_has_feat(S390_FEAT_GUARDED_STORAGE)) {
-mcic |= MCIC_VB_GS;
-}
-return mcic;
-}
-
 void kvm_s390_crw_mchk(void)
 {
 struct kvm_s390_irq irq = {
 .type = KVM_S390_MCHK,
-.u.mchk.cr14 = 1 << 28,
-.u.mchk.mcic = build_channel_report_mcic(),
+.u.mchk.cr14 = CR14_CHANNEL_REPORT_SC,
+.u.mchk.mcic = s390_build_validity_mcic() | MCIC_SC_CP,
 };
 kvm_s390_floating_interrupt(&irq);
 }
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 3/5] s390x/tcg: implement SET CLOCK PROGRAMMABLE FIELD

[David Hildenbrand]

Needed for machine check handling inside Linux (when restoring registers).

Except for SIGP and machine checks, we don't make use of the register
yet. Suficient for now.

Signed-off-by: David Hildenbrand 
---
 target/s390x/helper.h  |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/misc_helper.c | 11 +++
 target/s390x/translate.c   |  7 +++
 4 files changed, 21 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 9459b73c73..0281c286b8 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -127,6 +127,7 @@ DEF_HELPER_3(load_psw, noreturn, env, i64, i64)
 DEF_HELPER_FLAGS_2(spx, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(stck, TCG_CALL_NO_RWG_SE, i64, env)
 DEF_HELPER_FLAGS_2(sckc, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_1(sckpf, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(stckc, TCG_CALL_NO_RWG, i64, env)
 DEF_HELPER_FLAGS_2(spt, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_1(stpt, TCG_CALL_NO_RWG, i64, env)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 16e27c8a35..8c2541f545 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -999,6 +999,8 @@
 C(0xb204, SCK, S, Z,   0, 0, 0, 0, 0, 0)
 /* SET CLOCK COMPARATOR */
 C(0xb206, SCKC,S, Z,   0, m2_64, 0, 0, sckc, 0)
+/* SET CLOCK PROGRAMMABLE FIELD */
+C(0x0107, SCKPF,   E, Z,   0, 0, 0, 0, sckpf, 0)
 /* SET CPU TIMER */
 C(0xb208, SPT, S, Z,   0, m2_64, 0, 0, spt, 0)
 /* SET PREFIX */
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 6d766ce1e7..2c6ab329fb 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -146,6 +146,17 @@ void HELPER(sckc)(CPUS390XState *env, uint64_t time)
 timer_mod(env->tod_timer, env->tod_basetime + time);
 }
 
+/* Set Tod Programmable Field */
+void HELPER(sckpf)(CPUS390XState *env)
+{
+uint32_t val = env->regs[0];
+
+if (val & 0xUL) {
+s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+}
+env->todpr = val;
+}
+
 /* Store Clock Comparator */
 uint64_t HELPER(stckc)(CPUS390XState *env)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 26cf993405..48b031894a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3922,6 +3922,13 @@ static ExitStatus op_sckc(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_sckpf(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+gen_helper_sckpf(cpu_env);
+return NO_EXIT;
+}
+
 static ExitStatus op_stckc(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 for-2.12 0/5] s390x/tcg: CCW hotplug support

[David Hildenbrand]

On 04.12.2017 14:10, no-re...@patchew.org wrote:
> Hi,
> 
> This series failed build test on s390x host. Please find the details below.
> 
> Subject: [Qemu-devel] [PATCH v1 for-2.12 0/5] s390x/tcg: CCW hotplug support
> Type: series
> Message-id: 20171204125505.29203-1-da...@redhat.com

Based on s390x-next (cohuck)


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/3] s390x/css: unrestrict cssids

[Christian Borntraeger]

On 12/04/2017 12:10 PM, Cornelia Huck wrote:
[..]
> 
>>
>> Signed-off-by: Halil Pasic 

squashed or non-squashed:

Acked-by: Christian Borntraeger 

Re: [Qemu-devel] [Bug 1735384] Re: OpenJDK JVM segfaults on qemu-sh4 (regression)

[Alex Bennée]

John Paul Adrian Glaubitz  writes:

> On 12/04/2017 10:29 AM, Alex Bennée wrote:
>> It's hard to imagine a scenario where taking the tb_lock() for resolving
>> something that will fail is going to be an improvement. However maybe
>> there is a subtle difference with sh4's javavm implementation.
>
> So, OpenJDK doesn't have a SH-specific implementation of the JVM, it just
> uses the Zero variant, which is a pure C++ implementation of the JVM.
>
> The same implementation is used on any other architecture like older ARM
> (< ARMv7). I just tested it on ARMv4T and it doesn't crash there on
> qemu-user.
>
> However, SH4 is special due to its implementation of atomics in user
> space called gUSA for which support to qemu-user has been recently
> added by Richard Hendersson. Maybe the problem lies there.
>
>> A backtrace QEMU after the segv would be useful here.
>
> I forgot what the proper procedure is for running qemu-user inside
> GDB. Could you help me with that?

Either call directly:

  gdb --args qemu-foo 

Or alternatively:

  qemu-foo -g 1234 

And then:

  gdb qemu-foo -p 

And finally attaching to the gdbstub:

  gdb-multiarch -ex "target remote localhost:1234"
  c

Or just make sure your environment is generating core dumps you can
backtrace at leisure:

  gdb qemu-foo core
  bt


>
> The strace looks like this in any case:
>
> 28856 access("/etc/ld.so.nohwcap",F_OK) = -1 errno=2 (No such file or 
> directory)
> 28856 open("/lib/sh4-linux-gnu/libgcc_s.so.1",O_RDONLY|O_CLOEXEC) = 3
> 28856 read(3,0x7fffacd4,512) = 512
> 28856 fstat64(3,0x7fffabe8) = 0
> 28856 mmap(NULL,189084,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
> 0x7ee27000
> 28856 mprotect(0x7ee45000,61440,PROT_NONE) = 0
> 28856 
> mmap(0x7ee54000,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x1d000)
>  = 0x7ee54000
> 28856 close(3) = 0
> 28856 mprotect(0x7ee54000,4096,PROT_READ) = 0
> 28856 mprotect(0x7eee8000,4096,PROT_READ) = 0
> 28856 mprotect(0x7f05c000,20480,PROT_READ) = 0
> 28856 mprotect(0x7f5c8000,53248,PROT_READ) = 0
> 28856 getpid() = 28856
> 28856 munmap(0x7f065000,50134) = 0
> 28856 getpid() = 28856
> 28856 
> mmap(NULL,1572864,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|0x2,-1,0)
>  = 0x7eca7000
> 28856 mprotect(0x7eca7000,4096,PROT_NONE) = 0
> 28856 
> clone(CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,child_stack=0x7ee26048,parent_tidptr=0x7ee26528,tls=0x7ee26930,child_tidptr=0x7ee26528)
>  = 28860
> 28856 futex(0x7ee26528,FUTEX_WAIT,28860,NULL,0x7f77c6e8,2138556136)28856 
> set_robust_list(2128766256,12,-1,2128766652,-1,2128764832) = -1 errno=38 
> (Function not implemented)
> --- SIGSEGV {si_signo=SIGSEGV, si_code=1, si_addr=0x289da000} ---
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault
> (sid-sh4-sbuild)root@nofan:/local_scratch/sid-sh4-sbuild#
>
> Adrian
>
> --
>   .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
>`-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


--
Alex Bennée

[Qemu-devel] [PATCH v1 for-2.12 0/9] s390x/tcg: facilitites and instructions

[David Hildenbrand]

Wire up some io instructions and implement new facilitites. Make sure
to take care of MTTCG when it comes to atomic operations.

As we are now able to install/boot a Fedora 26/27 as well as an upstream
kernel compiled for z12, let's bump up the QEMU cpu model to a very
stripped down version of a z12 (with missing base features). Take care
of backwards compatibility (as we defined the QEMU model as
migration-safe).

Try it yourself: https://github.com/davidhildenbrand/qemu.git s390x-queue

This branch is based on https://github.com/cohuck/qemu.git s390x-next
and contains other patches sent previously, especially
- s390x/tcg: CCW hotplug support
- cpus: make pause_all_cpus() play with SMP on single threaded TCG
- cpu-exec: fix missed CPU kick during interrupt injection

$ 
baseurl=https://ftp-stud.hs-esslingen.de/pub/fedora-secondary/releases/27/Server/s390x/os/
$ wget ${baseurl}/images/kernel.img
$ wget ${baseurl}/images/initrd.img
$ qemu-img create -f qcow2 guest-tcg.qcow2 8G
$ qemu-system-s390x \
-nographic -machine s390-ccw-virtio -m 2048 \
--accel tcg,thread=multi -smp 4,maxcpus=4 \
-hda guest-tcg.qcow2 \
-kernel kernel.img \
-initrd initrd.img \
--append "TERM=linux inst.repo=${baseurl}/ ip=dhcp inst.geoloc=0"

Please note that we still have problems with floating interrupts. I have
already patches and will send them out soon, once the current set of
patches have been reviewed and picked up. So if you run into problems,
try with -smp 1.


David Hildenbrand (9):
  s390x/tcg: ALSI/ALSGI are atomic with interlocked-acccess facility 1
  s390x/tcg: implement Interlocked-Access Facility 2
  s390x/tcg: wire up SET ADDRESS LIMIT
  s390x/tcg: wire up SET CHANNEL MONITOR
  s390x/tcg: Implement STORE CHANNEL PATH STATUS
  s390x/tcg: Implement SIGA instruction
  s390x/tcg: implement extract-CPU-time facility
  s390x/tcg: we already implement the Set-Program-Parameter facility
  s390x: change the QEMU cpu model to a stripped down z12

 hw/s390x/s390-virtio-ccw.c  |  6 +++
 target/s390x/cpu.h  |  3 ++
 target/s390x/cpu_models.c   | 97 +++-
 target/s390x/cpu_models.h   |  1 +
 target/s390x/gen-features.c | 87 
 target/s390x/helper.h   |  2 +
 target/s390x/insn-data.def  | 22 ++
 target/s390x/misc_helper.c  | 18 +
 target/s390x/translate.c| 98 +
 9 files changed, 270 insertions(+), 64 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 2/9] s390x/tcg: implement Interlocked-Access Facility 2

[David Hildenbrand]

With this facility, OI/OIY, NI/NIY and XI/XIY are atomic. All operate on
one byte (MO_UB).

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu_models.c  |  1 +
 target/s390x/insn-data.def | 12 ++--
 target/s390x/translate.c   | 33 +
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index c4c37b3b15..94d24e423d 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -842,6 +842,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 S390_FEAT_STFLE_45,
 S390_FEAT_STFLE_49,
 S390_FEAT_LOCAL_TLB_CLEARING,
+S390_FEAT_INTERLOCKED_ACCESS_2,
 S390_FEAT_STFLE_53,
 S390_FEAT_MSA_EXT_5,
 S390_FEAT_MSA_EXT_3,
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 166ee7c80b..4e6dd6e348 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -99,8 +99,8 @@
 D(0xa505, NIHL,RI_a,  Z,   r1_o, i2_16u, r1, 0, andi, 0, 0x1020)
 D(0xa506, NILH,RI_a,  Z,   r1_o, i2_16u, r1, 0, andi, 0, 0x1010)
 D(0xa507, NILL,RI_a,  Z,   r1_o, i2_16u, r1, 0, andi, 0, 0x1000)
-C(0x9400, NI,  SI,Z,   m1_8u, i2_8u, new, m1_8, and, nz64)
-C(0xeb54, NIY, SIY,   LD,  m1_8u, i2_8u, new, m1_8, and, nz64)
+C(0x9400, NI,  SI,Z,   la1, i2_8u, new, 0, ni, nz64)
+C(0xeb54, NIY, SIY,   LD,  la1, i2_8u, new, 0, ni, nz64)
 
 /* BRANCH AND SAVE */
 C(0x0d00, BASR,RR_a,  Z,   0, r2_nz, r1, 0, bas, 0)
@@ -357,8 +357,8 @@
 /* EXCLUSIVE OR IMMEDIATE */
 D(0xc006, XIHF,RIL_a, EI,  r1_o, i2_32u, r1, 0, xori, 0, 0x2020)
 D(0xc007, XILF,RIL_a, EI,  r1_o, i2_32u, r1, 0, xori, 0, 0x2000)
-C(0x9700, XI,  SI,Z,   m1_8u, i2_8u, new, m1_8, xor, nz64)
-C(0xeb57, XIY, SIY,   LD,  m1_8u, i2_8u, new, m1_8, xor, nz64)
+C(0x9700, XI,  SI,Z,   la1, i2_8u, new, 0, xi, nz64)
+C(0xeb57, XIY, SIY,   LD,  la1, i2_8u, new, 0, xi, nz64)
 
 /* EXECUTE */
 C(0x4400, EX,  RX_a,  Z,   0, a2, 0, 0, ex, 0)
@@ -698,8 +698,8 @@
 D(0xa509, OIHL,RI_a,  Z,   r1_o, i2_16u, r1, 0, ori, 0, 0x1020)
 D(0xa50a, OILH,RI_a,  Z,   r1_o, i2_16u, r1, 0, ori, 0, 0x1010)
 D(0xa50b, OILL,RI_a,  Z,   r1_o, i2_16u, r1, 0, ori, 0, 0x1000)
-C(0x9600, OI,  SI,Z,   m1_8u, i2_8u, new, m1_8, or, nz64)
-C(0xeb56, OIY, SIY,   LD,  m1_8u, i2_8u, new, m1_8, or, nz64)
+C(0x9600, OI,  SI,Z,   la1, i2_8u, new, 0, oi, nz64)
+C(0xeb56, OIY, SIY,   LD,  la1, i2_8u, new, 0, oi, nz64)
 
 /* PACK */
 /* Really format SS_b, but we pack both lengths into one argument
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 79d2ee650c..edfe51b5c3 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1417,6 +1417,17 @@ static ExitStatus op_andi(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_ni(DisasContext *s, DisasOps *o)
+{
+o->in1 = tcg_temp_new_i64();
+/* Perform the atomic operation in memory. */
+tcg_gen_atomic_fetch_and_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
+ MO_UB);
+/* We need to recompute the operation for setting CC.  */
+tcg_gen_and_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_bas(DisasContext *s, DisasOps *o)
 {
 tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->next_pc));
@@ -3368,6 +3379,17 @@ static ExitStatus op_ori(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_oi(DisasContext *s, DisasOps *o)
+{
+o->in1 = tcg_temp_new_i64();
+/* Perform the atomic operation in memory. */
+tcg_gen_atomic_fetch_or_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
+MO_UB);
+/* We need to recompute the operation for setting CC.  */
+tcg_gen_or_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_pack(DisasContext *s, DisasOps *o)
 {
 TCGv_i32 l = tcg_const_i32(get_field(s->fields, l1));
@@ -4633,6 +4655,17 @@ static ExitStatus op_xori(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_xi(DisasContext *s, DisasOps *o)
+{
+o->in1 = tcg_temp_new_i64();
+/* Perform the atomic operation in memory. */
+tcg_gen_atomic_fetch_xor_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
+ MO_UB);
+/* We need to recompute the operation for setting CC.  */
+tcg_gen_xor_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_zero(DisasContext *s, DisasOps *o)
 {
 o->out = tcg_const_i64(0);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 1/9] s390x/tcg: ALSI/ALSGI are atomic with interlocked-acccess facility 1

[David Hildenbrand]

We can simply reuse our ASI implementation. Only the way CC is
calculated differs.

Signed-off-by: David Hildenbrand 
---
 target/s390x/insn-data.def | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 57f2e5133f..166ee7c80b 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -70,9 +70,9 @@
 C(0xc20b, ALFI,RIL_a, EI,  r1, i2_32u, new, r1_32, add, addu32)
 C(0xc20a, ALGFI,   RIL_a, EI,  r1, i2_32u, r1, 0, add, addu64)
 /* ADD LOGICAL WITH SIGNED IMMEDIATE */
-C(0xeb6e, ALSI,SIY,   GIE, m1_32u, i2, new, m1_32, add, addu32)
+D(0xeb6e, ALSI,SIY,   GIE, la1, i2, new, 0, asi, addu32, MO_TEUL)
 C(0xecda, ALHSIK,  RIE_d, DO,  r3, i2, new, r1_32, add, addu32)
-C(0xeb7e, ALGSI,   SIY,   GIE, m1_64, i2, new, m1_64, add, addu64)
+D(0xeb7e, ALGSI,   SIY,   GIE, la1, i2, new, 0, asi, addu64, MO_TEQ)
 C(0xecdb, ALGHSIK, RIE_d, DO,  r3, i2, r1, 0, add, addu64)
 /* ADD LOGICAL WITH SIGNED IMMEDIATE HIGH */
 C(0xcc0a, ALSIH,   RIL_a, HW,  r1_sr32, i2, new, r1_32h, add, addu32)
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 3/9] s390x/tcg: wire up SET ADDRESS LIMIT

[David Hildenbrand]

Let's handle it just like KVM:
Depending on the model, this instruction may not be
provided. When this instruction is not provided, it is
checked for operand exception and privileged-opera-
tion exception, and then is suppressed.

Signed-off-by: David Hildenbrand 
---
 target/s390x/helper.h  | 1 +
 target/s390x/insn-data.def | 1 +
 target/s390x/misc_helper.c | 9 +
 target/s390x/translate.c   | 7 +++
 4 files changed, 18 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 2ce57edc14..95ad44bc39 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -165,6 +165,7 @@ DEF_HELPER_2(hsch, void, env, i64)
 DEF_HELPER_3(msch, void, env, i64, i64)
 DEF_HELPER_2(rchp, void, env, i64)
 DEF_HELPER_2(rsch, void, env, i64)
+DEF_HELPER_2(sal, void, env, i64)
 DEF_HELPER_3(ssch, void, env, i64, i64)
 DEF_HELPER_2(stcrw, void, env, i64)
 DEF_HELPER_3(stsch, void, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 4e6dd6e348..8793350963 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1054,6 +1054,7 @@
 C(0xb232, MSCH,S, Z,   0, insn, 0, 0, msch, 0)
 C(0xb23b, RCHP,S, Z,   0, 0, 0, 0, rchp, 0)
 C(0xb238, RSCH,S, Z,   0, 0, 0, 0, rsch, 0)
+C(0xb237, SAL, S, Z,   0, 0, 0, 0, sal, 0)
 C(0xb233, SSCH,S, Z,   0, insn, 0, 0, ssch, 0)
 C(0xb239, STCRW,   S, Z,   0, insn, 0, 0, stcrw, 0)
 C(0xb234, STSCH,   S, Z,   0, insn, 0, 0, stsch, 0)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 55e78c56d2..d0d4441572 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -377,6 +377,15 @@ void HELPER(rsch)(CPUS390XState *env, uint64_t r1)
 qemu_mutex_unlock_iothread();
 }
 
+void HELPER(sal)(CPUS390XState *env, uint64_t r1)
+{
+S390CPU *cpu = s390_env_get_cpu(env);
+
+qemu_mutex_lock_iothread();
+ioinst_handle_sal(cpu, r1, GETPC());
+qemu_mutex_unlock_iothread();
+}
+
 void HELPER(ssch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
 S390CPU *cpu = s390_env_get_cpu(env);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index edfe51b5c3..a6346ac139 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4088,6 +4088,13 @@ static ExitStatus op_rsch(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_sal(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+gen_helper_sal(cpu_env, regs[1]);
+return NO_EXIT;
+}
+
 static ExitStatus op_ssch(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 8/9] s390x/tcg: we already implement the Set-Program-Parameter facility

[David Hildenbrand]

The Set-Program-Parameter facility (also known as Load-Program-Parameter
faciliy) provides the LPP instruction used to load the program
parameter. We already implement that instruction in TCG, so add it to our
list.

Note: Not documented in the PoP but in "The Load-Program-Parameter and
CPU-Measurement Facilities) - SA23-2260-05 document.

While at it, make the whole list ordered (accoring to cpu_features_def.h).

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu_models.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 0be037eac1..edac7fdecf 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -824,12 +824,12 @@ static void add_qemu_cpu_model_features(S390FeatBitmap 
fbm)
 S390_FEAT_IDTE_SEGMENT,
 S390_FEAT_STFLE,
 S390_FEAT_SENSE_RUNNING_STATUS,
-S390_FEAT_EXTENDED_IMMEDIATE,
 S390_FEAT_EXTENDED_TRANSLATION_2,
 S390_FEAT_MSA,
-S390_FEAT_EXTENDED_TRANSLATION_3,
 S390_FEAT_LONG_DISPLACEMENT,
 S390_FEAT_LONG_DISPLACEMENT_FAST,
+S390_FEAT_EXTENDED_IMMEDIATE,
+S390_FEAT_EXTENDED_TRANSLATION_3,
 S390_FEAT_ETF2_ENH,
 S390_FEAT_STORE_CLOCK_FAST,
 S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
@@ -839,6 +839,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
 S390_FEAT_EXECUTE_EXT,
+S390_FEAT_SET_PROGRAM_PARAMETERS,
 S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
 S390_FEAT_STFLE_45,
 S390_FEAT_STFLE_49,
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 7/9] s390x/tcg: implement extract-CPU-time facility

[David Hildenbrand]

It only provies the EXTRACT CPU TIME instruction. We can reuse the stpt
helper, which calculates the CPU timer value.

As the instruction is not priviledged, but we don't have a CPU timer
value in case of linux user, we simply fake the CPU timer to be 0.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu_models.c  |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/translate.c   | 36 
 3 files changed, 39 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 94d24e423d..0be037eac1 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -834,6 +834,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 S390_FEAT_STORE_CLOCK_FAST,
 S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
 S390_FEAT_ETF3_ENH,
+S390_FEAT_EXTRACT_CPU_TIME,
 S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
 S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index f7b66b0091..5e33bd27ff 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -369,6 +369,8 @@
 C(0xb24f, EAR, RRE,   Z,   0, 0, new, r1_32, ear, 0)
 /* EXTRACT CPU ATTRIBUTE */
 C(0xeb4c, ECAG,RSY_a, GIE, 0, a2, r1, 0, ecag, 0)
+/* EXTRACT CPU TIME */
+C(0xc801, ECTG,SSF,   ECT, 0, 0, 0, 0, ectg, 0)
 /* EXTRACT FPC */
 C(0xb38c, EFPC,RRE,   Z,   0, 0, new, r1_32, efpc, 0)
 /* EXTRACT PSW */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 1e4079464a..e0f55fc8e9 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3887,6 +3887,41 @@ static ExitStatus op_spm(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_ectg(DisasContext *s, DisasOps *o)
+{
+int b1 = get_field(s->fields, b1);
+int d1 = get_field(s->fields, d1);
+int b2 = get_field(s->fields, b2);
+int d2 = get_field(s->fields, d2);
+int r3 = get_field(s->fields, r3);
+TCGv_i64 tmp = tcg_temp_new_i64();
+
+/* fetch all operands first */
+o->in1 = tcg_temp_new_i64();
+tcg_gen_addi_i64(o->in1, regs[b1], d1);
+o->in2 = tcg_temp_new_i64();
+tcg_gen_addi_i64(o->in2, regs[b2], d2);
+o->addr1 = get_address(s, 0, r3, 0);
+
+/* load the third operand into r3 before modifying anything */
+tcg_gen_qemu_ld64(regs[r3], o->addr1, get_mem_index(s));
+
+#ifndef CONFIG_USER_ONLY
+/* subtract CPU timer from first operand and store in GR0 */
+gen_helper_stpt(tmp, cpu_env);
+tcg_gen_sub_i64(regs[0], o->in1, tmp);
+#else
+/* we don't have a CPU timer, fake value 0 */
+tcg_gen_mov_i64(regs[0], o->in1);
+#endif
+
+/* store second operand in GR1 */
+tcg_gen_mov_i64(regs[1], o->in2);
+
+tcg_temp_free_i64(tmp);
+return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_spka(DisasContext *s, DisasOps *o)
 {
@@ -5639,6 +5674,7 @@ enum DisasInsnEnum {
 #define FAC_MSA3S390_FEAT_MSA_EXT_3 /* msa-extension-3 facility */
 #define FAC_MSA4S390_FEAT_MSA_EXT_4 /* msa-extension-4 facility */
 #define FAC_MSA5S390_FEAT_MSA_EXT_5 /* msa-extension-5 facility */
+#define FAC_ECT S390_FEAT_EXTRACT_CPU_TIME
 
 static const DisasInsn insn_info[] = {
 #include "insn-data.def"
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 5/9] s390x/tcg: Implement STORE CHANNEL PATH STATUS

[David Hildenbrand]

Just like KVM does, we should suppress this instruction:
When this instruction is not provided, it is
checked for privileged operation exception and the
instruction is suppressed by the machine

Signed-off-by: David Hildenbrand 
---
 target/s390x/insn-data.def | 1 +
 target/s390x/translate.c   | 7 +++
 2 files changed, 8 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e21d226092..c7353e7f11 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1056,6 +1056,7 @@
 C(0xb238, RSCH,S, Z,   0, 0, 0, 0, rsch, 0)
 C(0xb237, SAL, S, Z,   0, 0, 0, 0, sal, 0)
 C(0xb23c, SCHM,S, Z,   0, insn, 0, 0, schm, 0)
+C(0xb23a, STCPS,   S, Z,   0, 0, 0, 0, stcps, 0)
 C(0xb233, SSCH,S, Z,   0, insn, 0, 0, ssch, 0)
 C(0xb239, STCRW,   S, Z,   0, insn, 0, 0, stcrw, 0)
 C(0xb234, STSCH,   S, Z,   0, insn, 0, 0, stsch, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index bd3fc6448e..5c2432678c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4102,6 +4102,13 @@ static ExitStatus op_schm(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_stcps(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+/* The instruction is suppressed if not provided. */
+return NO_EXIT;
+}
+
 static ExitStatus op_ssch(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 9/9] s390x: change the QEMU cpu model to a stripped down z12

[David Hildenbrand]

We are good enough to boot upstream Linux kernels / Fedora 26/27. That
should be suficient for now.

As the QEMU CPU model is migration safe, let's add compatibility code.
Generate the feature list to reduce the trouble of messing things up in the
future.

Signed-off-by: David Hildenbrand 
---
 hw/s390x/s390-virtio-ccw.c  |   6 +++
 target/s390x/cpu.h  |   3 ++
 target/s390x/cpu_models.c   | 100 ++--
 target/s390x/cpu_models.h   |   1 +
 target/s390x/gen-features.c |  87 ++
 5 files changed, 138 insertions(+), 59 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index a23b8aec9f..edd18b7fac 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -721,6 +721,9 @@ bool css_migration_enabled(void)
 
 static void ccw_machine_2_12_instance_options(MachineState *machine)
 {
+const S390FeatBitmap qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V2_12 };
+
+s390_set_qemu_cpu_model(0x2827, 12, 2, qemu_cpu_feat);
 }
 
 static void ccw_machine_2_12_class_options(MachineClass *mc)
@@ -730,7 +733,10 @@ DEFINE_CCW_MACHINE(2_12, "2.12", true);
 
 static void ccw_machine_2_11_instance_options(MachineState *machine)
 {
+const S390FeatBitmap qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V2_11 };
 ccw_machine_2_12_instance_options(machine);
+
+s390_set_qemu_cpu_model(0x2064, 7, 1, qemu_cpu_feat);
 }
 
 static void ccw_machine_2_11_class_options(MachineClass *mc)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 4d3e7920f5..969598b211 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -719,6 +719,9 @@ static inline unsigned int s390_cpu_set_state(uint8_t 
cpu_state, S390CPU *cpu)
 /* cpu_models.c */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 #define cpu_list s390_cpu_list
+void s390_set_qemu_cpu_model(uint16_t type, uint8_t gen, uint8_t ec_ga,
+ const S390FeatBitmap features);
+
 
 /* helper.c */
 #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index edac7fdecf..f0577f8155 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -15,7 +15,6 @@
 #include "internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
-#include "gen-features.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
@@ -81,6 +80,11 @@ static S390CPUDef s390_cpu_defs[] = {
 CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
 };
 
+#define QEMU_MAX_CPU_TYPE 0x2827
+#define QEMU_MAX_CPU_GEN 12
+#define QEMU_MAX_CPU_EC_GA 2
+static const S390FeatBitmap qemu_max_cpu_feat = { S390_FEAT_LIST_QEMU_MAX };
+
 /* features part of a base model but not relevant for finding a base model */
 S390FeatBitmap ignored_base_feat;
 
@@ -812,51 +816,6 @@ static void check_compatibility(const S390CPUModel 
*max_model,
   "available in the configuration: ");
 }
 
-/**
- * The base TCG CPU model "qemu" is based on the z900. However, we already
- * can also emulate some additional features of later CPU generations, so
- * we add these additional feature bits here.
- */
-static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
-{
-static const int feats[] = {
-S390_FEAT_DAT_ENH,
-S390_FEAT_IDTE_SEGMENT,
-S390_FEAT_STFLE,
-S390_FEAT_SENSE_RUNNING_STATUS,
-S390_FEAT_EXTENDED_TRANSLATION_2,
-S390_FEAT_MSA,
-S390_FEAT_LONG_DISPLACEMENT,
-S390_FEAT_LONG_DISPLACEMENT_FAST,
-S390_FEAT_EXTENDED_IMMEDIATE,
-S390_FEAT_EXTENDED_TRANSLATION_3,
-S390_FEAT_ETF2_ENH,
-S390_FEAT_STORE_CLOCK_FAST,
-S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
-S390_FEAT_ETF3_ENH,
-S390_FEAT_EXTRACT_CPU_TIME,
-S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
-S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
-S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
-S390_FEAT_EXECUTE_EXT,
-S390_FEAT_SET_PROGRAM_PARAMETERS,
-S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
-S390_FEAT_STFLE_45,
-S390_FEAT_STFLE_49,
-S390_FEAT_LOCAL_TLB_CLEARING,
-S390_FEAT_INTERLOCKED_ACCESS_2,
-S390_FEAT_STFLE_53,
-S390_FEAT_MSA_EXT_5,
-S390_FEAT_MSA_EXT_3,
-S390_FEAT_MSA_EXT_4,
-};
-int i;
-
-for (i = 0; i < ARRAY_SIZE(feats); i++) {
-set_bit(feats[i], fbm);
-}
-}
-
 static S390CPUModel *get_max_cpu_model(Error **errp)
 {
 static S390CPUModel max_model;
@@ -869,12 +828,10 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
 if (kvm_enabled()) {
 kvm_s390_get_host_cpu_model(&max_model, errp);
 } else {
-/* TCG emulates a z900 (with some optional additional features) */
-max_model.def = &s390_cpu_defs[0];
-bitmap_copy(max_model.features, max_model.def->default_feat,
-S390_FEAT_MAX);
-add_

[Qemu-devel] [PATCH v1 for-2.12 4/9] s390x/tcg: wire up SET CHANNEL MONITOR

[David Hildenbrand]

Let's just wire it up like KVM.

Signed-off-by: David Hildenbrand 
---
 target/s390x/helper.h  | 1 +
 target/s390x/insn-data.def | 1 +
 target/s390x/misc_helper.c | 9 +
 target/s390x/translate.c   | 7 +++
 4 files changed, 18 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 95ad44bc39..e5282b939c 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -166,6 +166,7 @@ DEF_HELPER_3(msch, void, env, i64, i64)
 DEF_HELPER_2(rchp, void, env, i64)
 DEF_HELPER_2(rsch, void, env, i64)
 DEF_HELPER_2(sal, void, env, i64)
+DEF_HELPER_4(schm, void, env, i64, i64, i64)
 DEF_HELPER_3(ssch, void, env, i64, i64)
 DEF_HELPER_2(stcrw, void, env, i64)
 DEF_HELPER_3(stsch, void, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 8793350963..e21d226092 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1055,6 +1055,7 @@
 C(0xb23b, RCHP,S, Z,   0, 0, 0, 0, rchp, 0)
 C(0xb238, RSCH,S, Z,   0, 0, 0, 0, rsch, 0)
 C(0xb237, SAL, S, Z,   0, 0, 0, 0, sal, 0)
+C(0xb23c, SCHM,S, Z,   0, insn, 0, 0, schm, 0)
 C(0xb233, SSCH,S, Z,   0, insn, 0, 0, ssch, 0)
 C(0xb239, STCRW,   S, Z,   0, insn, 0, 0, stcrw, 0)
 C(0xb234, STSCH,   S, Z,   0, insn, 0, 0, stsch, 0)
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index d0d4441572..98ecedb499 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -386,6 +386,15 @@ void HELPER(sal)(CPUS390XState *env, uint64_t r1)
 qemu_mutex_unlock_iothread();
 }
 
+void HELPER(schm)(CPUS390XState *env, uint64_t r1, uint64_t r2, uint64_t inst)
+{
+S390CPU *cpu = s390_env_get_cpu(env);
+
+qemu_mutex_lock_iothread();
+ioinst_handle_schm(cpu, r1, r2, inst >> 16, GETPC());
+qemu_mutex_unlock_iothread();
+}
+
 void HELPER(ssch)(CPUS390XState *env, uint64_t r1, uint64_t inst)
 {
 S390CPU *cpu = s390_env_get_cpu(env);
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a6346ac139..bd3fc6448e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4095,6 +4095,13 @@ static ExitStatus op_sal(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_schm(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+gen_helper_schm(cpu_env, regs[1], regs[2], o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_ssch(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 01/10] s390x/tcg: ASI/ASGI are atomic with interlocked-acccess facility 1

[David Hildenbrand]

The semantics of these operations changed. Let's implement them just
like LOAD AND ADD, so they are atomic.

This fixes random crashes when booting a Linux kernel compiled for
z196+ with SMP + MTTCG.

Signed-off-by: David Hildenbrand 
---

Whoops, forgot to include this patch as the first one.

 target/s390x/insn-data.def |  4 ++--
 target/s390x/translate.c   | 11 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 43ab1963c8..57f2e5133f 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -39,10 +39,10 @@
 C(0xb9d8, AHHLR,   RRF_a, HW,  r2_sr32, r3, new, r1_32h, add, adds32)
 /* ADD IMMEDIATE */
 C(0xc209, AFI, RIL_a, EI,  r1, i2, new, r1_32, add, adds32)
-C(0xeb6a, ASI, SIY,   GIE, m1_32s, i2, new, m1_32, add, adds32)
+D(0xeb6a, ASI, SIY,   GIE, la1, i2, new, 0, asi, adds32, MO_TESL)
 C(0xecd8, AHIK,RIE_d, DO,  r3, i2, new, r1_32, add, adds32)
 C(0xc208, AGFI,RIL_a, EI,  r1, i2, r1, 0, add, adds64)
-C(0xeb7a, AGSI,SIY,   GIE, m1_64, i2, new, m1_64, add, adds64)
+D(0xeb7a, AGSI,SIY,   GIE, la1, i2, new, 0, asi, adds64, MO_TEQ)
 C(0xecd9, AGHIK,   RIE_d, DO,  r3, i2, r1, 0, add, adds64)
 /* ADD IMMEDIATE HIGH */
 C(0xcc08, AIH, RIL_a, HW,  r1_sr32, i2, new, r1_32h, add, adds32)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5e051fdd03..79d2ee650c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1364,6 +1364,17 @@ static ExitStatus op_addc(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_asi(DisasContext *s, DisasOps *o)
+{
+o->in1 = tcg_temp_new_i64();
+/* Perform the atomic addition in memory. */
+tcg_gen_atomic_fetch_add_i64(o->in1, o->addr1, o->in2, get_mem_index(s),
+ s->insn->data);
+/* However, we need to recompute the addition for setting CC.  */
+tcg_gen_add_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_aeb(DisasContext *s, DisasOps *o)
 {
 gen_helper_aeb(o->out, cpu_env, o->in1, o->in2);
-- 
2.14.3

[Qemu-devel] [PATCH v1 for-2.12 6/9] s390x/tcg: Implement SIGA instruction

[David Hildenbrand]

I have no idea what that instruction does, but KVM seems to suppress it,
setting cc=3 (and as it seems to be an io instruction, it should be
protected). Let's do the same for TCG, so we're at least equal.

(it is used in the kernel for qdio, I wasn't even able to find the real
 name of that instruction)

Signed-off-by: David Hildenbrand 
---
 target/s390x/insn-data.def | 1 +
 target/s390x/translate.c   | 8 
 2 files changed, 9 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index c7353e7f11..f7b66b0091 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1056,6 +1056,7 @@
 C(0xb238, RSCH,S, Z,   0, 0, 0, 0, rsch, 0)
 C(0xb237, SAL, S, Z,   0, 0, 0, 0, sal, 0)
 C(0xb23c, SCHM,S, Z,   0, insn, 0, 0, schm, 0)
+C(0xb274, SIGA,S, Z,   0, 0, 0, 0, siga, 0)
 C(0xb23a, STCPS,   S, Z,   0, 0, 0, 0, stcps, 0)
 C(0xb233, SSCH,S, Z,   0, insn, 0, 0, ssch, 0)
 C(0xb239, STCRW,   S, Z,   0, insn, 0, 0, stcrw, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5c2432678c..1e4079464a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4102,6 +4102,14 @@ static ExitStatus op_schm(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_siga(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+/* From KVM code: Not provided, set CC = 3 for subchannel not operational 
*/
+gen_op_movi_cc(s, 3);
+return NO_EXIT;
+}
+
 static ExitStatus op_stcps(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
-- 
2.14.3

[Qemu-devel] [PATCH] linux-user: Fix locking order in fork_start()

[Peter Maydell]

Our locking order is that the tb lock should be taken
inside the mmap_lock, but fork_start() grabs locks the
other way around. This means that if a heavily multithreaded
guest process (such as Java) calls fork() it can deadlock,
with the thread that called fork() stuck in fork_start()
with the tb lock and waiting for the mmap lock, but some
other thread in tb_find() with the mmap lock and waiting
for the tb lock. The cpu_list_lock() should also always be
taken last, not first.

Fix this by making fork_start() grab the locks in the
right order. The order in which we drop locks doesn't
matter, so we leave fork_end() the way it is.

Signed-off-by: Peter Maydell 
Cc: qemu-sta...@nongnu.org
---
 linux-user/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 6286661..146ee3e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -128,9 +128,9 @@ int cpu_get_pic_interrupt(CPUX86State *env)
 /* Make sure everything is in a consistent state for calling fork().  */
 void fork_start(void)
 {
-cpu_list_lock();
-qemu_mutex_lock(&tb_ctx.tb_lock);
 mmap_fork_start();
+qemu_mutex_lock(&tb_ctx.tb_lock);
+cpu_list_lock();
 }
 
 void fork_end(int child)
-- 
2.7.4

Re: [Qemu-devel] [PULL 0/3] ppc-for-2.11 queue 20171204

[Peter Maydell]

On 4 December 2017 at 10:17, Peter Maydell  wrote:
> On 4 December 2017 at 03:47, David Gibson  wrote:
>> The following changes since commit c11d61271b9e6e7a1f0479ef1ca8fb55fa457a62:
>>
>>   Update version for v2.11.0-rc3 release (2017-11-29 17:59:34 +)
>>
>> are available in the Git repository at:
>>
>>   git://github.com/dgibson/qemu.git tags/ppc-for-2.11-20171204
>>
>> for you to fetch changes up to 768a20f3a491ed4afce73ebb65347d55251c0ebd:
>>
>>   spapr: Include "pre-plugged" DIMMS in ram size calculation at reset 
>> (2017-12-04 11:31:22 +1100)
>>
>> 
>> ppc patch queue 2017-12-04
>>
>> We are, alas, not yet to the bottom of ppc bugs.  This pull request
>> fixes several more.  I believe they're important enough to include in
>> 2.11. despite the late date.
>>
>> 
>> David Gibson (1):
>>   spapr: Include "pre-plugged" DIMMS in ram size calculation at reset
>>
>> Kurban Mallachiev (1):
>>   target-ppc: Don't invalidate non-supported msr bits
>>
>> Laurent Vivier (1):
>>   pseries: fix TCG migration
>
> So my question here is about the same as to mst: are these fixes all
> really vital? Were any of the bugs they fix present in 2.10 ?
>
> (A +7-4 diffstat is not too difficult to justify though.)

...so I looked at the patches and agreed they made sense for rc4.

Applied, thanks.
-- PMM

[Qemu-devel] [PATCH v2] spapr: fix LSI interrupt specifiers in the device tree

[Greg Kurz]

PAPR 2.7 C.6.9.1.2 describes the "#interrupt-cells" property of the
PowerPC External Interrupt Source Controller node as follows:

“#interrupt-cells”

  Standard property name to define the number of cells in an interrupt-
  specifier within an interrupt domain.

  prop-encoded-array: An integer, encoded as with encode-int, that denotes
  the number of cells required to represent an interrupt specifier in its
  child nodes.

  The value of this property for the PowerPC External Interrupt option shall
  be 2. Thus all interrupt specifiers (as used in the standard “interrupts”
  property) shall consist of two cells, each containing an integer encoded
  as with encode-int. The first integer represents the interrupt number the
  second integer is the trigger code: 0 for edge triggered, 1 for level
  triggered.

This patch fixes the interrupt specifiers in the "interrupt-map" property
of the PHB node, that were setting the second cell to 8 (confusion with
IRQ_TYPE_LEVEL_LOW ?) instead of 1.

VIO devices and RTAS event sources use the same format for interrupt
specifiers: while here, we introduce a common helper to handle the
encoding details.

Signed-off-by: Greg Kurz 
Reviewed-by: Cédric Le Goater 
Tested-by: Cédric Le Goater 
--
v2: - drop the erroneous changes to the "interrupts" prop in PCI device nodes
- introduce a common helper to encode interrupt specifiers
---
 hw/ppc/spapr_events.c  |3 +--
 hw/ppc/spapr_pci.c |5 +++--
 hw/ppc/spapr_vio.c |3 ++-
 include/hw/ppc/spapr.h |   11 +++
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 7dc87fc7bd77..c8205795b0c8 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -282,8 +282,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void *fdt)
 continue;
 }
 
-interrupts[0] = cpu_to_be32(source->irq);
-interrupts[1] = 0;
+spapr_dt_encode_interrupt_specifier(interrupts, source->irq, false);
 
 _FDT(node_offset = fdt_add_subnode(fdt, event_sources, source_name));
 _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 39134f0ef0a3..78769e833dd0 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2121,8 +2121,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
 irqmap[2] = 0;
 irqmap[3] = cpu_to_be32(j+1);
 irqmap[4] = cpu_to_be32(xics_phandle);
-irqmap[5] = cpu_to_be32(phb->lsi_table[lsi_num].irq);
-irqmap[6] = cpu_to_be32(0x8);
+spapr_dt_encode_interrupt_specifier(&irqmap[5],
+phb->lsi_table[lsi_num].irq,
+true);
 }
 }
 /* Write interrupt map */
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index bb7ed2c537b0..582b50a0443d 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -126,8 +126,9 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
 }
 
 if (dev->irq) {
-uint32_t ints_prop[] = {cpu_to_be32(dev->irq), 0};
+uint32_t ints_prop[2];
 
+spapr_dt_encode_interrupt_specifier(ints_prop, dev->irq, false);
 ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
   sizeof(ints_prop));
 if (ret < 0) {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 6b8e04c78771..423e72338309 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -590,6 +590,17 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, 
hwaddr addr);
 
 #define RTAS_EVENT_SCAN_RATE1
 
+/* This helper should be used to encode interrupt specifiers when the related
+ * "interrupt-controller" node has its "#interrupt-cells" property set to 2 
(ie,
+ * VIO devices, RTAS event sources and PHBs).
+ */
+static inline void spapr_dt_encode_interrupt_specifier(uint32_t *intspec,
+   int irq, bool is_lsi)
+{
+intspec[0] = cpu_to_be32(irq);
+intspec[1] = is_lsi ? cpu_to_be32(1) : 0;
+}
+
 typedef struct sPAPRTCETable sPAPRTCETable;
 
 #define TYPE_SPAPR_TCE_TABLE "spapr-tce-table"

Re: [Qemu-devel] [PATCH RFC 0/2] s390x: cut down on unattached devices

[Halil Pasic]

On 12/04/2017 10:22 AM, Cornelia Huck wrote:
> On Fri, 1 Dec 2017 15:41:21 +0100
> Halil Pasic  wrote:
> 
>> On 11/28/2017 04:21 PM, Halil Pasic wrote:
>> [..]
> Otherwise at first glance both patches seem sane.  

 Can I count this as an ack, or do you plan to do more review?
  
>>>
>>> Yes I was planning to give it another look. And I do already
>>> have questions. Isn't the QOM composition tree API? I mean
>>> let's assume the QMP commands working on this tree are not completely
>>> useless. How is client code (management software) supposed to work,
>>> assumed it can rely on paths of e.g. properties being stable. Just
>>> imagine we had this default-cssid property (for the sake of the
>>> argument, not like we want it) on the css bridge.  
>>
>> Ping! I would like to get this clarified before proceeding with reviewing
>> this series.
> 
> [It might be helpful to not drop cc:s.]
> 

Sorry. Wrong button.

> I don't think we really want a static tree. As long as the devices are
> locateable, it should be fine.
> 

What do you mean by locateable in particular?

Let's say I'm management software guy who want's to access a certain
property of a certain device. For that I'm supposed to use qom-get. Now
qom-get takes a path as input argument (absolute or relative). The question
is, how I'm supposed to figure out this path as management software developer?
In other words what is the algorithm?

One naive approach would be, to assume that the path is stable between
releases. So I have to figure it out when I'm implementing the stuff,
and then it keeps working ever after. I read your answer as this naive
approach is wrong.

(BTW I find static also confusing in this context.)

[..]

Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Max Reitz]

On 2017-12-03 21:13, Daniel Henrique Barboza wrote:
> Hi Max,
> 
> On 12/01/2017 06:13 PM, Max Reitz wrote:
>> On 2017-11-16 23:35, Daniel Henrique Barboza wrote:
>>> This patch implements a test case for the scenario that was failing
>>> prior to the patch "migration/ram.c: do not set 'postcopy_running' in
>>> POSTCOPY_INCOMING_END".
>>>
>>> This new test file 198 was derived from the test file 181 authored
>>> by Kevin Wolf.
>>>
>>> CC: Kevin Wolf 
>>> CC: Max Reitz 
>>> CC: Cleber Rosa 
>>> Signed-off-by: Daniel Henrique Barboza 
>>>
>>> ---
>>> I CCed Cleber Rosa because this patch was developed at the same
>>> time his patch series "[PATCH 00/10] I/O tests cleanups" hit the
>>> mailing list. Most of the cleanups/fixes he made in the series was
>>> done in this new test as well.
>>>
>>>  tests/qemu-iotests/198 | 110 
>>> +
>>>  tests/qemu-iotests/198.out |  20 +
>>>  tests/qemu-iotests/group   |   1 +
>>>  3 files changed, 131 insertions(+)
>>>  create mode 100755 tests/qemu-iotests/198
>>>  create mode 100644 tests/qemu-iotests/198.out
>> Looks good overall, just two nitpicks below.
>>
>>> diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
>>> new file mode 100755
>>> index 00..786e37fc95
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/198
>>> @@ -0,0 +1,110 @@
>>> +#!/bin/bash
>>> +#
>>> +# Test savevm and loadvm after live migration with postcopy flag
>>> +#
>>> +# Copyright (C) 2017, IBM Corporation.
>>> +#
>>> +# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
>>> +#
>>> +# 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 : danie...@linux.vnet.ibm.com
>>> +
>>> +seq=`basename $0`
>>> +echo "QA output created by $seq"
>>> +
>>> +status=1   # failure is the default!
>>> +
>>> +MIG_SOCKET="${TEST_DIR}/migrate"
>>> +
>>> +# get standard environment, filters and checks
>>> +. ./common.rc
>>> +. ./common.filter
>>> +. ./common.qemu
>>> +
>>> +_cleanup()
>>> +{
>>> +rm -f "${MIG_SOCKET}"
>>> +_cleanup_test_img
>>> +_cleanup_qemu
>>> +}
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_supported_fmt qcow2
>>> +_supported_proto generic
>>> +_supported_os Linux
>>> +
>>> +size=64M
>>> +_make_test_img $size
>>> +
>>> +echo
>>> +echo === Starting VMs ===
>>> +echo
>>> +
>>> +qemu_comm_method="monitor"
>>> +
>>> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
>>> +_launch_qemu \
>>> +-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
>>> +else
>>> +_launch_qemu \
>>> +-drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
>>> +fi
>>> +src=$QEMU_HANDLE
>>> +
>>> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
>>> +_launch_qemu \
>>> +-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
>>> +-incoming "unix:${MIG_SOCKET}"
>>> +else
>>> +_launch_qemu \
>>> +-drive 
>>> file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
>>> +-incoming "unix:${MIG_SOCKET}"
>>> +fi
>>> +dest=$QEMU_HANDLE
>>> +
>>> +echo
>>> +echo === Set \'migrate_set_capability postcopy-ram on\' and migrate ===
>>> +echo
>>> +
>>> +silent=yes
>>> +_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
>>> +_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
>>> +_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
>>> +
>>> +QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
>>> +_send_qemu_cmd $src "info migrate" "completed\|failed"
>>> +silent=yes _send_qemu_cmd $src "" "(qemu)"
>> Would it make sense to query the migration status non-silently here,
>> too, so that the test output would verify that it has actually succeeded?
>>
>> (I suppose the savevm/loadvm coming up next would fail if the migration
>> failed, but maybe if it's easy enough...)
> 
> Not exactly non-silently, but we can do something like this:
> 
> 
> echo
> echo === Check if migration was successful ===
> echo
> 
> QEMU_COMM_TIMEOUT=1 silent=yes \
>     _send_qemu_cmd $src "info migrate" "completed"
> silent=yes _send_qemu_cmd $src "" "(qemu)"
> 
> 
> If migration failed, the test will fail with timeout error waiting for
> "completed"
> prior to savevm. What do you think?

Sounds good.  Shouldn't you keep the qemu_cmd_repeat=10, though?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 for-2.12 10/25] block: Fix bdrv_find_backing_image()

[Alberto Garcia]

On Mon 20 Nov 2017 09:09:49 PM CET, Max Reitz wrote:
> bdrv_find_backing_image() should use bdrv_get_full_backing_filename() or
> bdrv_make_absolute_filename() instead of trying to do what those
> functions do by itself.
>
> path_combine_deprecated() can now be dropped, so let's do that.
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH v7 for-2.12 14/25] block/nbd: Make bdrv_dirname() return NULL

[Alberto Garcia]

On Mon 20 Nov 2017 09:09:53 PM CET, Max Reitz wrote:
> The generic bdrv_dirname() implementation would be able to generate some
> form of directory name for many NBD nodes, but it would be always wrong.
> Therefore, we have to explicitly make it an error (until NBD has some
> form of specification for export paths, if it ever will).
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH 1/3] s390x/css: unrestrict cssids

[Halil Pasic]

On 12/04/2017 12:10 PM, Cornelia Huck wrote:
> On Fri,  1 Dec 2017 15:31:34 +0100
> Halil Pasic  wrote:
> 
>> The default css 0xfe is currently restricted to virtual subchannel
>> devices. The hope when the decision was made was, that non-virtual
>> subchannel devices will come around when guests can exploit multiple
>> channel subsystems. Since current guests don't do that, the pain of the
>> partitioned (cssid) namespace outweighs the gain.
>>
>> The default css 0xfe is currently restricted to virtual subchannel
>> devices. The hope when the decision was made was, that non-virtual
>> subchannel devices will come around when guest can exploit multiple
>> channel subsystems. Since the guests generally don't do, the pain
>> of the partitioned (cssid) namespace outweighs the gain.
> 
> Doubled paragraph?
> 

Yep. Copy paste mistake.

>>
>> Let us remove the corresponding restrictions (virtual devices
>> can be put only in 0xfe and non-virtual devices in any css except
>> the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).
>>
>> At the same time, change our schema for generating css bus ids to put
>> both virtual and non-virtual devices into the default css (spilling over
>> into other css images, if needed). The intention is to deprecate
>> s390-squash-mcss. Whit this change devices without a specified devno
> 
> s/Whit/With/

Nod

> 
>> won't end up hidden to guests not supporting multiple channel subsystems,
>> unless this can not be avoided (default css full).
>>
>> Deprecaton of s390-squash-mcss and indicating the changes via QMP is

s/Deprecaton/Deprecation/

>> expected to follow soon (as separate commits).
> 
> Let's drop this paragraph (the qmp interface should be squashed in, and
> you mention the deprecation right above.)
> 
>>
>> The adverse effect of getting rid of the restriction on migration should
>> not be too severe.  Vfio-ccw devices are not live-migratable yet, and for
>> virtual devices using the extra freedom would only make sense with the
>> aforementioned guest support in place.
>>
>> The auto-generated bus ids are affected by both changes. We hope to not
>> encounter any auto-generated bus ids in production as Libvirt is always
>> explicit about the bus id.  Since 8ed179c937 ("s390x/css: catch section
>> mismatch on load", 2017-05-18) the worst that can happen because the same
>> device ended up having a different bus id is a cleanly failed migration.
>> I find it hard to reason about the impact of changed auto-generated bus
>> ids on migration for command line users as I don't know which rules is
>> such an user supposed to follow.
> 
> Should we document somewhere that guests supposed to be migrated should
> make sure that they use explicit devnos?
> 

I think having a document collecting such migration rules and best practices
for command line users (and implicitly also for implementers of management
software) would be a good idea. Maybe there is such a documentation, but
I don't know where. The devnos should be a part of it for sure. But I'm
not volunteering for creating this kind of documentation. Natural languages
aren't my forte.

>>
>> Another pain-point is down- or upgrade of QEMU for command line users.
>> The old way and the new way of doing vfio-ccw are mutually incompatible.
>> Libvirt is only going to support the new way, so for libvirt users, the
>> possible problems at QEMU downgrade are the following. If a domain
>> contains virtual devices placed into a css different than 0xfe the domain
>> will refuse to start with a QEMU not having this patch. Putting devices
>> into a css different that 0xfe however won't make much sense in the near
>> future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU
>> not having this patch. This is business as usual.
> 
> My writing style would be to have this as a shorter, bulleted list -
> but no need to rewrite this if this is understandable to the others on
> cc:
> 

If you want, we can iterate on the description. My primary concern was
to agree on how to advertise this change.

>>
>> Signed-off-by: Halil Pasic 
>>
>> ---
>> Hi!
>>
>> I've factored out the announcing via QMP interface stuff to ease review.
>> I would not mind the two being squashed together before this hits main,
>> as I would much prefer having the two as one (atomic) change. But the
>> second part turned out so controversial, that splitting is expected to
>> benefit the review process.
>>
>> v2 -> v3:
>> * factored out announcing into a separate patch
>> * reworded commit message
>> * removed outdated comment about squash
>>
>> v1 -> v2:
>> * changed ccw bus id generation too (see commit message)
>> * moved the property to the machine (see cover letter)
>> * added a description to the property
>> ---
>>  hw/s390x/3270-ccw.c|  2 +-
>>  hw/s390x/css.c | 28 
>>  hw/s390x/s390-ccw.c|  2 +-
>>  hw/s390x/s390-virtio-ccw.c |  1 -
>>  hw/s390x/virtio-ccw.c  |  2 +-
>>  include/hw

Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Max Reitz]

On 2017-12-04 16:01, Daniel Henrique Barboza wrote:
> 
> 
> On 12/04/2017 12:50 PM, Max Reitz wrote:
>> On 2017-12-03 21:13, Daniel Henrique Barboza wrote:
>>> Hi Max,
>>>
>>> On 12/01/2017 06:13 PM, Max Reitz wrote:
 On 2017-11-16 23:35, Daniel Henrique Barboza wrote:
> This patch implements a test case for the scenario that was failing
> prior to the patch "migration/ram.c: do not set 'postcopy_running' in
> POSTCOPY_INCOMING_END".
>
> This new test file 198 was derived from the test file 181 authored
> by Kevin Wolf.
>
> CC: Kevin Wolf 
> CC: Max Reitz 
> CC: Cleber Rosa 
> Signed-off-by: Daniel Henrique Barboza 
>
> ---
> I CCed Cleber Rosa because this patch was developed at the same
> time his patch series "[PATCH 00/10] I/O tests cleanups" hit the
> mailing list. Most of the cleanups/fixes he made in the series was
> done in this new test as well.
>
>   tests/qemu-iotests/198 | 110
> +
>   tests/qemu-iotests/198.out |  20 +
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 131 insertions(+)
>   create mode 100755 tests/qemu-iotests/198
>   create mode 100644 tests/qemu-iotests/198.out
 Looks good overall, just two nitpicks below.

> diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
> new file mode 100755
> index 00..786e37fc95
> --- /dev/null
> +++ b/tests/qemu-iotests/198
> @@ -0,0 +1,110 @@
> +#!/bin/bash
> +#
> +# Test savevm and loadvm after live migration with postcopy flag
> +#
> +# Copyright (C) 2017, IBM Corporation.
> +#
> +# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
> +#
> +# 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 : danie...@linux.vnet.ibm.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +status=1    # failure is the default!
> +
> +MIG_SOCKET="${TEST_DIR}/migrate"
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +_cleanup()
> +{
> +    rm -f "${MIG_SOCKET}"
> +    _cleanup_test_img
> +    _cleanup_qemu
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_supported_fmt qcow2
> +_supported_proto generic
> +_supported_os Linux
> +
> +size=64M
> +_make_test_img $size
> +
> +echo
> +echo === Starting VMs ===
> +echo
> +
> +qemu_comm_method="monitor"
> +
> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
> +    _launch_qemu \
> +    -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
> +else
> +    _launch_qemu \
> +    -drive
> file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
> +fi
> +src=$QEMU_HANDLE
> +
> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
> +    _launch_qemu \
> +    -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
> +    -incoming "unix:${MIG_SOCKET}"
> +else
> +    _launch_qemu \
> +    -drive
> file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
> +    -incoming "unix:${MIG_SOCKET}"
> +fi
> +dest=$QEMU_HANDLE
> +
> +echo
> +echo === Set \'migrate_set_capability postcopy-ram on\' and
> migrate ===
> +echo
> +
> +silent=yes
> +_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on'
> "(qemu)"
> +_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
> +_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
> +
> +QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
> +    _send_qemu_cmd $src "info migrate" "completed\|failed"
> +silent=yes _send_qemu_cmd $src "" "(qemu)"
 Would it make sense to query the migration status non-silently here,
 too, so that the test output would verify that it has actually
 succeeded?

 (I suppose the savevm/loadvm coming up next would fail if the migration
 failed, but maybe if it's easy enough...)
>>> Not exactly non-silently, but we can do something like this:
>>>
>>>
>>> echo
>>> echo === Check if migration was successful

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Halil Pasic]

On 12/04/2017 12:15 PM, Cornelia Huck wrote:
> On Fri,  1 Dec 2017 15:31:35 +0100
> Halil Pasic  wrote:
> 
>> Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
>> to the management software (so it can tell are cssids unrestricted or
>> restricted).
>>
>> Signed-off-by: Halil Pasic 
>> ---
>>
>> Boris says having the property on the virtual-css-bridge is good form
>> Libvirt PoV. @Shalini: could you verify that things work out fine
>> (provided we get at least a preliminary blessing from Connie).
>>
>> Consider squashing into "s390x/css: unrestrict cssids".
>> ---
>>  hw/s390x/css-bridge.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
>> index c4a9735d71..c7e8998680 100644
>> --- a/hw/s390x/css-bridge.c
>> +++ b/hw/s390x/css-bridge.c
>> @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
>>  DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> +static bool prop_get_true(Object *obj, Error **errp)
>> +{
>> +return true;
>> +}
>> +
>>  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
>>  {
>>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
>> *klass, void *data)
>>  hc->unplug = ccw_device_unplug;
>>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>>  dc->props = virtual_css_bridge_properties;
>> +object_class_property_add_bool(klass, "cssid-unrestricted",
>> +   prop_get_true, NULL, NULL);
>> +object_class_property_set_description(klass, "cssid-unrestricted",
>> +"A css device can use any  cssid, regardless whether virtual"
> 
> extra space -^
> 

Nod.

>> +" or not (read only, always true)",

Do we need "." here ^ ?

>> +NULL);
>>  }
>>  
>>  static const TypeInfo virtual_css_bridge_info = {
> 
> Looks reasonable. If this works as expected, I'll squash it into the
> previous patch.
> 

I've just asked Shalini to verify the libvirt perspective.

Supposed we verify this works as expected, I read I don't have to spin
a v2 and you are going to fix the issues found yourself. Right?

Re: [Qemu-devel] [PATCH v7 for-2.12 17/25] iotests: Add quorum case to test 110

[Alberto Garcia]

On Mon 20 Nov 2017 09:09:56 PM CET, Max Reitz wrote:
> Test 110 tests relative backing filenames for complex BDS trees.  Now
> that the originally supposedly failing test passes, let us add a new
> failing test: Quorum can never work automatically (without detecting
> whether all child nodes have the same base directory, but that would be
> rather inconsistent behavior).
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Daniel Henrique Barboza]

On 12/04/2017 12:50 PM, Max Reitz wrote:

On 2017-12-03 21:13, Daniel Henrique Barboza wrote:

Hi Max,

On 12/01/2017 06:13 PM, Max Reitz wrote:

On 2017-11-16 23:35, Daniel Henrique Barboza wrote:

This patch implements a test case for the scenario that was failing
prior to the patch "migration/ram.c: do not set 'postcopy_running' in
POSTCOPY_INCOMING_END".

This new test file 198 was derived from the test file 181 authored
by Kevin Wolf.

CC: Kevin Wolf 
CC: Max Reitz 
CC: Cleber Rosa 
Signed-off-by: Daniel Henrique Barboza 

---
I CCed Cleber Rosa because this patch was developed at the same
time his patch series "[PATCH 00/10] I/O tests cleanups" hit the
mailing list. Most of the cleanups/fixes he made in the series was
done in this new test as well.

  tests/qemu-iotests/198 | 110 +
  tests/qemu-iotests/198.out |  20 +
  tests/qemu-iotests/group   |   1 +
  3 files changed, 131 insertions(+)
  create mode 100755 tests/qemu-iotests/198
  create mode 100644 tests/qemu-iotests/198.out

Looks good overall, just two nitpicks below.


diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
new file mode 100755
index 00..786e37fc95
--- /dev/null
+++ b/tests/qemu-iotests/198
@@ -0,0 +1,110 @@
+#!/bin/bash
+#
+# Test savevm and loadvm after live migration with postcopy flag
+#
+# Copyright (C) 2017, IBM Corporation.
+#
+# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
+#
+# 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 : danie...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+MIG_SOCKET="${TEST_DIR}/migrate"
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_cleanup()
+{
+rm -f "${MIG_SOCKET}"
+_cleanup_test_img
+_cleanup_qemu
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+size=64M
+_make_test_img $size
+
+echo
+echo === Starting VMs ===
+echo
+
+qemu_comm_method="monitor"
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+_launch_qemu \
+-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
+else
+_launch_qemu \
+-drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
+fi
+src=$QEMU_HANDLE
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+_launch_qemu \
+-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
+-incoming "unix:${MIG_SOCKET}"
+else
+_launch_qemu \
+-drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
+-incoming "unix:${MIG_SOCKET}"
+fi
+dest=$QEMU_HANDLE
+
+echo
+echo === Set \'migrate_set_capability postcopy-ram on\' and migrate ===
+echo
+
+silent=yes
+_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
+_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
+_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
+
+QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
+_send_qemu_cmd $src "info migrate" "completed\|failed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"

Would it make sense to query the migration status non-silently here,
too, so that the test output would verify that it has actually succeeded?

(I suppose the savevm/loadvm coming up next would fail if the migration
failed, but maybe if it's easy enough...)

Not exactly non-silently, but we can do something like this:


echo
echo === Check if migration was successful ===
echo

QEMU_COMM_TIMEOUT=1 silent=yes \
     _send_qemu_cmd $src "info migrate" "completed"
silent=yes _send_qemu_cmd $src "" "(qemu)"


If migration failed, the test will fail with timeout error waiting for
"completed"
prior to savevm. What do you think?

Sounds good.  Shouldn't you keep the qemu_cmd_repeat=10, though?


I don't think it's needed. The command prior to it was:

+QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
+_send_qemu_cmd $src "info migrate" "completed\|failed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"


So it's guaranteed that the next 'info migrate' will contain the string
"completed" or "failed".


Daniel




Max

Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Daniel Henrique Barboza]

On 12/04/2017 01:06 PM, Max Reitz wrote:

On 2017-12-04 16:01, Daniel Henrique Barboza wrote:


On 12/04/2017 12:50 PM, Max Reitz wrote:

On 2017-12-03 21:13, Daniel Henrique Barboza wrote:

Hi Max,

On 12/01/2017 06:13 PM, Max Reitz wrote:

On 2017-11-16 23:35, Daniel Henrique Barboza wrote:

This patch implements a test case for the scenario that was failing
prior to the patch "migration/ram.c: do not set 'postcopy_running' in
POSTCOPY_INCOMING_END".

This new test file 198 was derived from the test file 181 authored
by Kevin Wolf.

CC: Kevin Wolf 
CC: Max Reitz 
CC: Cleber Rosa 
Signed-off-by: Daniel Henrique Barboza 

---
I CCed Cleber Rosa because this patch was developed at the same
time his patch series "[PATCH 00/10] I/O tests cleanups" hit the
mailing list. Most of the cleanups/fixes he made in the series was
done in this new test as well.

   tests/qemu-iotests/198 | 110
+
   tests/qemu-iotests/198.out |  20 +
   tests/qemu-iotests/group   |   1 +
   3 files changed, 131 insertions(+)
   create mode 100755 tests/qemu-iotests/198
   create mode 100644 tests/qemu-iotests/198.out

Looks good overall, just two nitpicks below.


diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
new file mode 100755
index 00..786e37fc95
--- /dev/null
+++ b/tests/qemu-iotests/198
@@ -0,0 +1,110 @@
+#!/bin/bash
+#
+# Test savevm and loadvm after live migration with postcopy flag
+#
+# Copyright (C) 2017, IBM Corporation.
+#
+# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
+#
+# 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 : danie...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1    # failure is the default!
+
+MIG_SOCKET="${TEST_DIR}/migrate"
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_cleanup()
+{
+    rm -f "${MIG_SOCKET}"
+    _cleanup_test_img
+    _cleanup_qemu
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+size=64M
+_make_test_img $size
+
+echo
+echo === Starting VMs ===
+echo
+
+qemu_comm_method="monitor"
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+    _launch_qemu \
+    -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
+else
+    _launch_qemu \
+    -drive
file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
+fi
+src=$QEMU_HANDLE
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+    _launch_qemu \
+    -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
+    -incoming "unix:${MIG_SOCKET}"
+else
+    _launch_qemu \
+    -drive
file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
+    -incoming "unix:${MIG_SOCKET}"
+fi
+dest=$QEMU_HANDLE
+
+echo
+echo === Set \'migrate_set_capability postcopy-ram on\' and
migrate ===
+echo
+
+silent=yes
+_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on'
"(qemu)"
+_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
+_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
+
+QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
+    _send_qemu_cmd $src "info migrate" "completed\|failed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"

Would it make sense to query the migration status non-silently here,
too, so that the test output would verify that it has actually
succeeded?

(I suppose the savevm/loadvm coming up next would fail if the migration
failed, but maybe if it's easy enough...)

Not exactly non-silently, but we can do something like this:


echo
echo === Check if migration was successful ===
echo

QEMU_COMM_TIMEOUT=1 silent=yes \
  _send_qemu_cmd $src "info migrate" "completed"
silent=yes _send_qemu_cmd $src "" "(qemu)"


If migration failed, the test will fail with timeout error waiting for
"completed"
prior to savevm. What do you think?

Sounds good.  Shouldn't you keep the qemu_cmd_repeat=10, though?

I don't think it's needed. The command prior to it was:

+QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
+    _send_qemu_cmd $src "info migrate" "completed\|failed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"


So it's guaranteed that the next 'info migrate' will contain the string
"completed" or "failed".

Ah, right.  I thought you'd simply replace that by the other.  But
either way works for me.

Hmmm that's a good approach too I guess

[Qemu-devel] [PULL 0/1] baum braille device update

[Samuel Thibault]

warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
The following changes since commit 7edaf99759017d3e175e37cffc3536e86a3bd380:

  Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into 
staging (2017-11-13 13:54:59 +)

are available in the Git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 15405525fce773d43e7e5cabc2b9ac4b9d4d7672:

  baum: Truncate braille device size to 84x1 (2017-12-03 19:35:50 +0100)


baum updates


Samuel Thibault (1):
  baum: Truncate braille device size to 84x1

 chardev/baum.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

[Qemu-devel] [PULL 1/1] baum: Truncate braille device size to 84x1

[Samuel Thibault]

Baum device bigger than 84 do not actually exist, some guest drivers
would be upset by such sizes.

Signed-off-by: Samuel Thibault 
---
 chardev/baum.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/chardev/baum.c b/chardev/baum.c
index 67fd783a59..78b0c87625 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -1,7 +1,7 @@
 /*
  * QEMU Baum Braille Device
  *
- * Copyright (c) 2008, 2010-2011, 2016 Samuel Thibault
+ * Copyright (c) 2008, 2010-2011, 2016-2017 Samuel Thibault
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
@@ -239,6 +239,12 @@ static int baum_deferred_init(BaumChardev *baum)
 brlapi_perror("baum: brlapi__getDisplaySize");
 return 0;
 }
+if (baum->y > 1) {
+baum->y = 1;
+}
+if (baum->x > 84) {
+baum->x = 84;
+}
 
 con = qemu_console_lookup_by_index(0);
 if (con && qemu_console_is_graphic(con)) {
-- 
2.15.0

[Qemu-devel] [PATCH v4 0/1] savevm/loadvm with postcopy flag qemu-iotest

[Daniel Henrique Barboza]

Changes from v3:
- rebased with master - file name changed from 198 to 201
- removed Creator/owner information (it can be retrieved with git)
- added a migration successful check before executing savevm
- removed a trailing double comma in the snapshot name


Daniel Henrique Barboza (1):
  tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

 tests/qemu-iotests/201 | 116 +
 tests/qemu-iotests/201.out |  23 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 140 insertions(+)
 create mode 100755 tests/qemu-iotests/201
 create mode 100644 tests/qemu-iotests/201.out

-- 
2.13.6

[Qemu-devel] [PATCH v4 1/1] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Daniel Henrique Barboza]

This patch implements a test case for the scenario that was failing
prior to the patch "migration/ram.c: do not set 'postcopy_running' in
POSTCOPY_INCOMING_END", commit acab30b85d.

This new test file 201 was derived from the test file 181 authored
by Kevin Wolf.

CC: Kevin Wolf 
CC: Max Reitz 
CC: Cleber Rosa 
Signed-off-by: Daniel Henrique Barboza 
---
 tests/qemu-iotests/201 | 116 +
 tests/qemu-iotests/201.out |  23 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 140 insertions(+)
 create mode 100755 tests/qemu-iotests/201
 create mode 100644 tests/qemu-iotests/201.out

diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201
new file mode 100755
index 00..9b6e23bbfc
--- /dev/null
+++ b/tests/qemu-iotests/201
@@ -0,0 +1,116 @@
+#!/bin/bash
+#
+# Test savevm and loadvm after live migration with postcopy flag
+#
+# Copyright (C) 2017, IBM Corporation.
+#
+# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
+#
+# 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 .
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+MIG_SOCKET="${TEST_DIR}/migrate"
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_cleanup()
+{
+rm -f "${MIG_SOCKET}"
+_cleanup_test_img
+_cleanup_qemu
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+size=64M
+_make_test_img $size
+
+echo
+echo === Starting VMs ===
+echo
+
+qemu_comm_method="monitor"
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+_launch_qemu \
+-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
+else
+_launch_qemu \
+-drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
+fi
+src=$QEMU_HANDLE
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+_launch_qemu \
+-drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
+-incoming "unix:${MIG_SOCKET}"
+else
+_launch_qemu \
+-drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
+-incoming "unix:${MIG_SOCKET}"
+fi
+dest=$QEMU_HANDLE
+
+echo
+echo === Set \'migrate_set_capability postcopy-ram on\' and migrate ===
+echo
+
+silent=yes
+_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
+_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
+_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
+
+QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
+_send_qemu_cmd $src "info migrate" "completed\|failed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"
+
+echo
+echo === Check if migration was successful ===
+echo
+
+QEMU_COMM_TIMEOUT=1 silent=yes \
+_send_qemu_cmd $src "info migrate" "completed"
+silent=yes _send_qemu_cmd $src "" "(qemu)"
+
+echo
+echo === On destination, execute savevm and loadvm ===
+echo
+
+silent=
+_send_qemu_cmd $dest 'savevm state1' "(qemu)"
+_send_qemu_cmd $dest 'loadvm state1' "(qemu)"
+
+echo
+echo === Shut down and check image ===
+echo
+
+_send_qemu_cmd $src 'quit' ""
+_send_qemu_cmd $dest 'quit' ""
+wait=1 _cleanup_qemu
+
+_check_test_img
+
+# success, all done
+echo "*** done"
+status=0
diff --git a/tests/qemu-iotests/201.out b/tests/qemu-iotests/201.out
new file mode 100644
index 00..9faf06f996
--- /dev/null
+++ b/tests/qemu-iotests/201.out
@@ -0,0 +1,23 @@
+QA output created by 201
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+
+=== Starting VMs ===
+
+
+=== Set 'migrate_set_capability postcopy-ram on' and migrate ===
+
+
+=== Check if migration was successful ===
+
+
+=== On destination, execute savevm and loadvm ===
+
+(qemu) savevm state1
+(qemu) loadvm state1
+
+=== Shut down and check image ===
+
+(qemu) quit
+(qemu) quit
+No errors were found on the image.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3e688678dd..c16bc2c5e2 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -197,3 +197,4 @@
 197 rw auto quick
 198 rw auto
 200 rw auto
+201 rw auto migration
-- 
2.13.6

Re: [Qemu-devel] [PULL 0/1] baum braille device update

[Peter Maydell]

On 4 December 2017 at 15:17, Samuel Thibault
 wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 7edaf99759017d3e175e37cffc3536e86a3bd380:
>
>   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into 
> staging (2017-11-13 13:54:59 +)
>
> are available in the Git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 15405525fce773d43e7e5cabc2b9ac4b9d4d7672:
>
>   baum: Truncate braille device size to 84x1 (2017-12-03 19:35:50 +0100)
>
> 
> baum updates
>
> 
> Samuel Thibault (1):
>   baum: Truncate braille device size to 84x1

Hi. Is this pull request intended for 2.11? We're currently
in freeze for that and only taking "absolutely release critical"
bug fixes...

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] baum braille device update

[Samuel Thibault]

Hello,

Peter Maydell, on lun. 04 déc. 2017 15:21:48 +, wrote:
> On 4 December 2017 at 15:17, Samuel Thibault
>  wrote:
> > warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> > The following changes since commit 7edaf99759017d3e175e37cffc3536e86a3bd380:
> >
> >   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into 
> > staging (2017-11-13 13:54:59 +)
> >
> > are available in the Git repository at:
> >
> >   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
> >
> > for you to fetch changes up to 15405525fce773d43e7e5cabc2b9ac4b9d4d7672:
> >
> >   baum: Truncate braille device size to 84x1 (2017-12-03 19:35:50 +0100)
> >
> > 
> > baum updates
> >
> > 
> > Samuel Thibault (1):
> >   baum: Truncate braille device size to 84x1
> 
> Hi. Is this pull request intended for 2.11?

If possible, yes.

> We're currently in freeze for that and only taking "absolutely release
> critical" bug fixes...

Ok, as you wish.

It's a relatively corner case: a friend of mine does have a 88-cell
braille display, and can not use it with guests by running qemu
-usbdevice braille due to this bug.

Samuel

Re: [Qemu-devel] [PATCH 21/25] spapr: introduce a helper to map the XIVE memory regions

[Cédric Le Goater]

On 12/04/2017 08:52 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:51PM +0100, Cédric Le Goater wrote:
>> When the XIVE interrupt mode is activated, the machine needs to expose
>> to the guest the MMIO regions use by the controller :
>>
>>   - Event State Buffer (ESB)
>>   - Thread Interrupt Management Area (TIMA)
>>
>> Migration will also need to reflect the current interrupt mode in use.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive_hcall.c  | 14 ++
>>  hw/ppc/spapr.c  |  5 +
>>  include/hw/ppc/spapr_xive.h |  1 +
>>  3 files changed, 20 insertions(+)
>>
>> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c
>> index 60c6c9f4be8f..ba217144878e 100644
>> --- a/hw/intc/spapr_xive_hcall.c
>> +++ b/hw/intc/spapr_xive_hcall.c
>> @@ -933,3 +933,17 @@ void spapr_xive_populate(sPAPRMachineState *spapr, int 
>> nr_servers,
>>  _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
>>   plat_res_int_priorities, 
>> sizeof(plat_res_int_priorities)));
>>  }
>> +
>> +void spapr_xive_mmio_map(sPAPRMachineState *spapr)
>> +{
>> +sPAPRXive *xive = spapr->xive;
>> +
>> +/* ESBs */
>> +sysbus_mmio_map(SYS_BUS_DEVICE(xive), 0, xive->esb_base);
>> +
>> +/* Thread Management Interrupt Areas */
>> +/* TODO: Only map the OS TIMA for the moment. Mapping the whole
>> + * region needs some rework in the handlers */
>> +sysbus_mmio_map(SYS_BUS_DEVICE(xive), 1,
>> +xive->tm_base + (1 << xive->tm_shift));
> 
> You probably shouldn't be exposing the user TIMA in the DT if you're
> only allowing the OS TIME to be mapped.

The specs requires to map both Uset and OS TIMA.

> 
>> +}
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3a62369883cc..734706c18cb3 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1132,6 +1132,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>>  } else {
>>  /* Populate device tree for XIVE */
>>  spapr_xive_populate(spapr, xics_max_server_number(), fdt, 
>> PHANDLE_XICP);
>> +spapr_xive_mmio_map(spapr);
> 
> This doesn't belong here, spapr_build_fdt() should _just_ build the
> fdt, not have side effects on the actual device state.

Yes. I will move the rest of the XIVE setup in the reset handler
before the device tree is built.

Thanks,

C.  

>>  }
>>  
>>  ret = spapr_populate_memory(spapr, fdt);
>> @@ -1613,6 +1614,10 @@ static int spapr_post_load(void *opaque, int 
>> version_id)
>>  }
>>  }
>>  
>> +if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
>> +spapr_xive_mmio_map(spapr);
>> +}
>> +
>>  return err;
>>  }
>>  
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index f6d4bf26e06a..88355f7eb643 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -84,5 +84,6 @@ typedef struct sPAPRMachineState sPAPRMachineState;
>>  void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>>  void spapr_xive_populate(sPAPRMachineState *spapr, int nr_servers, void 
>> *fdt,
>>   uint32_t phandle);
>> +void spapr_xive_mmio_map(sPAPRMachineState *spapr);
>>  
>>  #endif /* PPC_SPAPR_XIVE_H */
> 

Re: [Qemu-devel] [PATCH 3/3] s390x: deprecate s390-squash-mcss machine prop

[Halil Pasic]

On 12/04/2017 12:28 PM, Cornelia Huck wrote:
> On Fri,  1 Dec 2017 15:31:36 +0100
> Halil Pasic  wrote:
> 
>> With the cssids unrestricted (commit  "s390x/css: unrestrict
> 
> I won't add the commit id, as I intend to merge this in one go and I
> don't want to edit this every time I rebase prior to pull.
> 

I'm fine with dropping the commit id.

>> cssids") the s390-squash-mcss machine property should not be used.
>> Actually libvirt never supported this, so the expectation is that
>> removing it should be pretty painless.  But let's play nice and deprecate
>> it first.
>>
>> Signed-off-by: Halil Pasic 
>> ---
>>
>> I would like to reference a commit for "s390x/css: unrestrict cssids"
>> which is currently in RFC status on the list.
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 6 +-
>>  qemu-doc.texi  | 8 
>>  qemu-options.hx| 8 +++-
>>  3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 4d65a50334..3796f666e6 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, 
>> bool value,
>>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>  
>>  ms->s390_squash_mcss = value;
>> +if (ms->s390_squash_mcss) {
>> +warn_report("The machine property 's390-squash-mcss' is deprecated"
>> +" (obsoleted by lifting the cssid restrictions).");
> 
> Too bad that we can't warn when this is explicitly set to false - OTOH
> I don't really expect anyone doing that.
> 

I did not really dig deep. It may be possible, but I had other priorities,
and was not sure if it's worth (as I also don't expect it being set to false
explicit in the wild).

>> +}
>>  }
>>  
>>  static inline void s390_machine_initfn(Object *obj)
>> @@ -582,7 +586,7 @@ static inline void s390_machine_initfn(Object *obj)
>>  object_property_add_bool(obj, "s390-squash-mcss",
>>   machine_get_squash_mcss,
>>   machine_set_squash_mcss, NULL);
>> -object_property_set_description(obj, "s390-squash-mcss",
>> +object_property_set_description(obj, "s390-squash-mcss", "(deprecated) "
>>  "enable/disable squashing subchannels into the default css",
>>  NULL);
>>  object_property_set_bool(obj, false, "s390-squash-mcss", NULL);
>> diff --git a/qemu-doc.texi b/qemu-doc.texi
>> index db2351c746..874432d87c 100644
>> --- a/qemu-doc.texi
>> +++ b/qemu-doc.texi
>> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>>  
>>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>>  
>> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
>> +
>> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
>> +cssid to be chosen freely. Instead of squashing subchannels into the
>> +default channel subsystem image for guests that do not support multiple
>> +channel subsystems, all devices can be put into the default channel
>> +subsystem image.
>> +
>>  @section qemu-img command line arguments
>>  
>>  @subsection convert -s (since 2.0.0)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index f11c4ac960..fe0c29271f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -43,7 +43,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>  "suppress-vmdesc=on|off disables self-describing 
>> migration (default=off)\n"
>>  "nvdimm=on|off controls NVDIMM support (default=off)\n"
>>  "enforce-config-section=on|off enforce configuration 
>> section migration (default=off)\n"
>> -"s390-squash-mcss=on|off controls support for squashing 
>> into default css (default=off)\n",
>> +"s390-squash-mcss=on|off (deprecated) controls support 
>> for squashing into default css (default=off)\n",
>>  QEMU_ARCH_ALL)
>>  STEXI
>>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>> @@ -98,6 +98,12 @@ Enables or disables NVDIMM support. The default is off.
>>  @item s390-squash-mcss=on|off
>>  Enables or disables squashing subchannels into the default css.
>>  The default is off.
>> +NOTE: This property is deprecated and will be removed in future releases.
>> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
>> +cssid to be chosen freely. Instead of squashing subchannels into the
>> +default channel subsystem image for guests that do not support multiple
>> +channel subsystems, all devices can be put into the default channel
>> +subsystem image.
>>  @item enforce-config-section=on|off
>>  If @option{enforce-config-section} is set to @var{on}, force migration
>>  code to send configuration section even if the machine-type sets the
> 
> Looks sane. We should put a note into the 2.12 changelog as well.
> 

I agree. Who would be responsible for updat

Re: [Qemu-devel] [PULL 0/1] baum braille device update

[Peter Maydell]

On 4 December 2017 at 15:26, Samuel Thibault  wrote:
> Hello,
>
> Peter Maydell, on lun. 04 déc. 2017 15:21:48 +, wrote:
>> On 4 December 2017 at 15:17, Samuel Thibault
>>  wrote:
>> > 
>> > Samuel Thibault (1):
>> >   baum: Truncate braille device size to 84x1
>>
>> Hi. Is this pull request intended for 2.11?
>
> If possible, yes.
>
>> We're currently in freeze for that and only taking "absolutely release
>> critical" bug fixes...
>
> Ok, as you wish.
>
> It's a relatively corner case: a friend of mine does have a 88-cell
> braille display, and can not use it with guests by running qemu
> -usbdevice braille due to this bug.

Mmm. Looking at the code for baum.c for 2.10, this isn't a
regression from 2.10 or a security issue, which is what it
would need to be for us to want to put this in at this point.

Also, looking in my mail archives I couldn't find where
this patch was initially posted on the list for review. Our
process is that all patches have to be posted for review first,
they can't just go straight into a pull request.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-img: Fixed grammatical error in dump_human_image_check

[Fam Zheng]

On Sat, 12/02 14:37, Shravan Rajinikanth wrote:
> Signed-off-by: Shravan Rajinikanth 
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 68b375f..bea9268 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -580,7 +580,7 @@ static void dump_human_image_check(ImageCheck *check, 
> bool quiet)
>  if (check->leaks) {
>  qprintf(quiet,
>  "\n%" PRId64 " leaked clusters were found on the 
> image.\n"
> -"This means waste of disk space, but no harm to data.\n",
> +"This means disk space is wasted, but data is safe.\n",

To me both versions seem fine, could you explain the grammatical error in the
old message?

Fam

>  check->leaks);
>  }
>  
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [PATCH] qemu-img: Fixed grammatical error in dump_human_image_check

[Max Reitz]

On 2017-12-02 23:37, Shravan Rajinikanth wrote:
> Signed-off-by: Shravan Rajinikanth 
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 68b375f..bea9268 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -580,7 +580,7 @@ static void dump_human_image_check(ImageCheck *check, 
> bool quiet)
>  if (check->leaks) {
>  qprintf(quiet,
>  "\n%" PRId64 " leaked clusters were found on the 
> image.\n"
> -"This means waste of disk space, but no harm to data.\n",
> +"This means disk space is wasted, but data is safe.\n",
>  check->leaks);
>  }

How exactly is this a grammatical error?  (I'm not a native English
speaker, but it always seemed perfectly OK to me)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qemu-img: Fixed grammatical error in dump_human_image_check

[Peter Maydell]

On 4 December 2017 at 15:51, Max Reitz  wrote:
> On 2017-12-02 23:37, Shravan Rajinikanth wrote:
>> Signed-off-by: Shravan Rajinikanth 
>> ---
>>  qemu-img.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 68b375f..bea9268 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -580,7 +580,7 @@ static void dump_human_image_check(ImageCheck *check, 
>> bool quiet)
>>  if (check->leaks) {
>>  qprintf(quiet,
>>  "\n%" PRId64 " leaked clusters were found on the 
>> image.\n"
>> -"This means waste of disk space, but no harm to 
>> data.\n",
>> +"This means disk space is wasted, but data is safe.\n",
>>  check->leaks);
>>  }
>
> How exactly is this a grammatical error?  (I'm not a native English
> speaker, but it always seemed perfectly OK to me)

I think "This means" more naturally takes a verb phrase, not a noun phrase.
I don't know that I'd go so far as to say that the current text is
ungrammatical, but I do think the proposed change sounds more natural to me.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/3] s390x/css: unrestrict cssids

[Cornelia Huck]

On Mon, 4 Dec 2017 16:02:14 +0100
Halil Pasic  wrote:

> On 12/04/2017 12:10 PM, Cornelia Huck wrote:
> > On Fri,  1 Dec 2017 15:31:34 +0100
> > Halil Pasic  wrote:
> >   
> >> The default css 0xfe is currently restricted to virtual subchannel
> >> devices. The hope when the decision was made was, that non-virtual
> >> subchannel devices will come around when guests can exploit multiple
> >> channel subsystems. Since current guests don't do that, the pain of the
> >> partitioned (cssid) namespace outweighs the gain.
> >>
> >> The default css 0xfe is currently restricted to virtual subchannel
> >> devices. The hope when the decision was made was, that non-virtual
> >> subchannel devices will come around when guest can exploit multiple
> >> channel subsystems. Since the guests generally don't do, the pain
> >> of the partitioned (cssid) namespace outweighs the gain.  
> > 
> > Doubled paragraph?
> >   
> 
> Yep. Copy paste mistake.
> 
> >>
> >> Let us remove the corresponding restrictions (virtual devices
> >> can be put only in 0xfe and non-virtual devices in any css except
> >> the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).
> >>
> >> At the same time, change our schema for generating css bus ids to put
> >> both virtual and non-virtual devices into the default css (spilling over
> >> into other css images, if needed). The intention is to deprecate
> >> s390-squash-mcss. Whit this change devices without a specified devno  
> > 
> > s/Whit/With/  
> 
> Nod
> 
> >   
> >> won't end up hidden to guests not supporting multiple channel subsystems,
> >> unless this can not be avoided (default css full).
> >>
> >> Deprecaton of s390-squash-mcss and indicating the changes via QMP is  
> 
> s/Deprecaton/Deprecation/
> 
> >> expected to follow soon (as separate commits).  
> > 
> > Let's drop this paragraph (the qmp interface should be squashed in, and
> > you mention the deprecation right above.)
> >   
> >>
> >> The adverse effect of getting rid of the restriction on migration should
> >> not be too severe.  Vfio-ccw devices are not live-migratable yet, and for
> >> virtual devices using the extra freedom would only make sense with the
> >> aforementioned guest support in place.
> >>
> >> The auto-generated bus ids are affected by both changes. We hope to not
> >> encounter any auto-generated bus ids in production as Libvirt is always
> >> explicit about the bus id.  Since 8ed179c937 ("s390x/css: catch section
> >> mismatch on load", 2017-05-18) the worst that can happen because the same
> >> device ended up having a different bus id is a cleanly failed migration.
> >> I find it hard to reason about the impact of changed auto-generated bus
> >> ids on migration for command line users as I don't know which rules is
> >> such an user supposed to follow.  
> > 
> > Should we document somewhere that guests supposed to be migrated should
> > make sure that they use explicit devnos?
> >   
> 
> I think having a document collecting such migration rules and best practices
> for command line users (and implicitly also for implementers of management
> software) would be a good idea. Maybe there is such a documentation, but
> I don't know where. The devnos should be a part of it for sure. But I'm
> not volunteering for creating this kind of documentation. Natural languages
> aren't my forte.

I would not mind someone else doing this.

> 
> >>
> >> Another pain-point is down- or upgrade of QEMU for command line users.
> >> The old way and the new way of doing vfio-ccw are mutually incompatible.
> >> Libvirt is only going to support the new way, so for libvirt users, the
> >> possible problems at QEMU downgrade are the following. If a domain
> >> contains virtual devices placed into a css different than 0xfe the domain
> >> will refuse to start with a QEMU not having this patch. Putting devices
> >> into a css different that 0xfe however won't make much sense in the near
> >> future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU
> >> not having this patch. This is business as usual.  
> > 
> > My writing style would be to have this as a shorter, bulleted list -
> > but no need to rewrite this if this is understandable to the others on
> > cc:
> >   
> 
> If you want, we can iterate on the description. My primary concern was
> to agree on how to advertise this change.

Let's skip that.

> 
> >>
> >> Signed-off-by: Halil Pasic 

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Cornelia Huck]

On Mon, 4 Dec 2017 16:07:27 +0100
Halil Pasic  wrote:

> On 12/04/2017 12:15 PM, Cornelia Huck wrote:
> > On Fri,  1 Dec 2017 15:31:35 +0100
> > Halil Pasic  wrote:
> >   
> >> Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
> >> to the management software (so it can tell are cssids unrestricted or
> >> restricted).
> >>
> >> Signed-off-by: Halil Pasic 
> >> ---
> >>
> >> Boris says having the property on the virtual-css-bridge is good form
> >> Libvirt PoV. @Shalini: could you verify that things work out fine
> >> (provided we get at least a preliminary blessing from Connie).
> >>
> >> Consider squashing into "s390x/css: unrestrict cssids".
> >> ---
> >>  hw/s390x/css-bridge.c | 11 +++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
> >> index c4a9735d71..c7e8998680 100644
> >> --- a/hw/s390x/css-bridge.c
> >> +++ b/hw/s390x/css-bridge.c
> >> @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
> >>  DEFINE_PROP_END_OF_LIST(),
> >>  };
> >>  
> >> +static bool prop_get_true(Object *obj, Error **errp)
> >> +{
> >> +return true;
> >> +}
> >> +
> >>  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
> >>  {
> >>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> >> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
> >> *klass, void *data)
> >>  hc->unplug = ccw_device_unplug;
> >>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >>  dc->props = virtual_css_bridge_properties;
> >> +object_class_property_add_bool(klass, "cssid-unrestricted",
> >> +   prop_get_true, NULL, NULL);
> >> +object_class_property_set_description(klass, "cssid-unrestricted",
> >> +"A css device can use any  cssid, regardless whether virtual" 
> >>  
> > 
> > extra space -^
> >   
> 
> Nod.
> 
> >> +" or not (read only, always true)",  
> 
> Do we need "." here ^ ?

None of the other descs do that.

> 
> >> +NULL);
> >>  }
> >>  
> >>  static const TypeInfo virtual_css_bridge_info = {  
> > 
> > Looks reasonable. If this works as expected, I'll squash it into the
> > previous patch.
> >   
> 
> I've just asked Shalini to verify the libvirt perspective.
> 
> Supposed we verify this works as expected, I read I don't have to spin
> a v2 and you are going to fix the issues found yourself. Right?

I'd prefer a v2 that I can simply apply. Let's wait for some more
acks/r-bs.

Re: [Qemu-devel] [PATCH 15/25] spapr: notify the CPU when the XIVE interrupt priority is more privileged

[Benjamin Herrenschmidt]

On Mon, 2017-12-04 at 12:17 +1100, David Gibson wrote:
> On Sat, Dec 02, 2017 at 08:40:58AM -0600, Benjamin Herrenschmidt wrote:
> > On Thu, 2017-11-30 at 16:00 +1100, David Gibson wrote:
> > > 
> > > >  static uint64_t spapr_xive_icp_accept(sPAPRXiveICP *icp)
> > > >  {
> > > > -return 0;
> > > > +uint8_t nsr = icp->tima_os[TM_NSR];
> > > > +
> > > > +qemu_irq_lower(icp->output);
> > > > +
> > > > +if (icp->tima_os[TM_NSR] & TM_QW1_NSR_EO) {
> > > > +uint8_t cppr = icp->tima_os[TM_PIPR];
> > > > +
> > > > +icp->tima_os[TM_CPPR] = cppr;
> > > > +
> > > > +/* Reset the pending buffer bit */
> > > > +icp->tima_os[TM_IPB] &= ~priority_to_ipb(cppr);
> > > 
> > > What if multiple irqs of the same priority were queued?
> > 
> > It's the job of the OS to handle that case by consuming from the queue
> > until it's empty. There is an MMIO the guest can use if it wants to
> > that can set the IPB bits back to 1 for a given priority. Otherwise in
> > Linux we just have a SW way to force a replay.
> 
> Ok, so "accept" is effectively saying the OS is accepting all
> interrupts from that queue, right?

It's whatever you want it to mean. It's simply a test & clear on the
prio bit. From a HW standpoint, you could have multiple queues or just
set an internal SW flag to go chck again later etc...

> 
> > 
> > > > +icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
> > > > +
> > > > +/* Drop Exception bit for OS */
> > > > +icp->tima_os[TM_NSR] &= ~TM_QW1_NSR_EO;
> > > > +}
> > > > +
> > > > +return (nsr << 8) | icp->tima_os[TM_CPPR];
> > > > +}
> > > > +
> > > > +static void spapr_xive_icp_notify(sPAPRXiveICP *icp)
> > > > +{
> > > > +if (icp->tima_os[TM_PIPR] < icp->tima_os[TM_CPPR]) {
> > > > +icp->tima_os[TM_NSR] |= TM_QW1_NSR_EO;
> > > > +qemu_irq_raise(icp->output);
> > > > +}
> > > >  }
> > > >  
> > > >  static void spapr_xive_icp_set_cppr(sPAPRXiveICP *icp, uint8_t cppr)
> > > > @@ -51,6 +105,9 @@ static void spapr_xive_icp_set_cppr(sPAPRXiveICP 
> > > > *icp, uint8_t cppr)
> > > >  }
> > > >  
> > > >  icp->tima_os[TM_CPPR] = cppr;
> > > > +
> > > > +/* CPPR has changed, inform the ICP which might raise an exception 
> > > > */
> > > > +spapr_xive_icp_notify(icp);
> > > >  }
> > > >  
> > > >  /*
> > > > @@ -224,6 +281,8 @@ static void spapr_xive_irq(sPAPRXive *xive, int 
> > > > lisn)
> > > >  XiveEQ *eq;
> > > >  uint32_t eq_idx;
> > > >  uint8_t priority;
> > > > +uint32_t server;
> > > > +sPAPRXiveICP *icp;
> > > >  
> > > >  ive = spapr_xive_get_ive(xive, lisn);
> > > >  if (!ive || !(ive->w & IVE_VALID)) {
> > > > @@ -253,6 +312,13 @@ static void spapr_xive_irq(sPAPRXive *xive, int 
> > > > lisn)
> > > >  qemu_log_mask(LOG_UNIMP, "XIVE: !UCOND_NOTIFY not 
> > > > implemented\n");
> > > >  }
> > > >  
> > > > +server = GETFIELD(EQ_W6_NVT_INDEX, eq->w6);
> > > > +icp = spapr_xive_icp_get(xive, server);
> > > > +if (!icp) {
> > > > +qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No ICP for server %d\n", 
> > > > server);
> > > > +return;
> > > > +}
> > > > +
> > > >  if (GETFIELD(EQ_W6_FORMAT_BIT, eq->w6) == 0) {
> > > >  priority = GETFIELD(EQ_W7_F0_PRIORITY, eq->w7);
> > > >  
> > > > @@ -260,9 +326,18 @@ static void spapr_xive_irq(sPAPRXive *xive, int 
> > > > lisn)
> > > >  if (priority == 0xff) {
> > > >  g_assert_not_reached();
> > > >  }
> > > > +
> > > > +/* Update the IPB (Interrupt Pending Buffer) with the priority
> > > > + * of the new notification and inform the ICP, which will
> > > > + * decide to raise the exception, or not, depending the CPPR.
> > > > + */
> > > > +icp->tima_os[TM_IPB] |= priority_to_ipb(priority);
> > > > +icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]);
> > > >  } else {
> > > >  qemu_log_mask(LOG_UNIMP, "XIVE: w7 format1 not implemented\n");
> > > >  }
> > > > +
> > > > +spapr_xive_icp_notify(icp);
> > > >  }
> > > >  
> > > >  /*
> > > 
> > > 
> 
> 

Re: [Qemu-devel] [PULL 0/1] baum braille device update

[Samuel Thibault]

Peter Maydell, on lun. 04 déc. 2017 15:36:32 +, wrote:
> On 4 December 2017 at 15:26, Samuel Thibault  wrote:
> > Peter Maydell, on lun. 04 déc. 2017 15:21:48 +, wrote:
> >> On 4 December 2017 at 15:17, Samuel Thibault
> >>  wrote:
> >> > 
> >> > Samuel Thibault (1):
> >> >   baum: Truncate braille device size to 84x1
> >>
> >> Hi. Is this pull request intended for 2.11?
> >
> > If possible, yes.
> >
> >> We're currently in freeze for that and only taking "absolutely release
> >> critical" bug fixes...
> >
> > Ok, as you wish.
> >
> > It's a relatively corner case: a friend of mine does have a 88-cell
> > braille display, and can not use it with guests by running qemu
> > -usbdevice braille due to this bug.
> 
> Mmm. Looking at the code for baum.c for 2.10, this isn't a
> regression from 2.10 or a security issue, which is what it
> would need to be for us to want to put this in at this point.

Ah, ok, nevermind then, it'll have to wait.

Sorry I didn't know that.

Samuel

Re: [Qemu-devel] [PATCH 3/3] s390x: deprecate s390-squash-mcss machine prop

[Cornelia Huck]

On Mon, 4 Dec 2017 16:32:16 +0100
Halil Pasic  wrote:

> On 12/04/2017 12:28 PM, Cornelia Huck wrote:
> > On Fri,  1 Dec 2017 15:31:36 +0100
> > Halil Pasic  wrote:

> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 4d65a50334..3796f666e6 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object 
> >> *obj, bool value,
> >>  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> >>  
> >>  ms->s390_squash_mcss = value;
> >> +if (ms->s390_squash_mcss) {
> >> +warn_report("The machine property 's390-squash-mcss' is 
> >> deprecated"
> >> +" (obsoleted by lifting the cssid restrictions).");  
> > 
> > Too bad that we can't warn when this is explicitly set to false - OTOH
> > I don't really expect anyone doing that.
> >   
> 
> I did not really dig deep. It may be possible, but I had other priorities,
> and was not sure if it's worth (as I also don't expect it being set to false
> explicit in the wild).

It's not worth spending any more time on this.

> 
> >> +}
> >>  }
> >>  
> >>  static inline void s390_machine_initfn(Object *obj)

(...)

> > Looks sane. We should put a note into the 2.12 changelog as well.
> >   
> 
> I agree. Who would be responsible for updating the changelog. I'm not
> familiar with that process yet. To be honest, I wouldn't mind having
> the changelog notice in your writing style. Guess would be better for
> everyone ;).

Just a one-liner once we have the 2.12 changelog page.

> 
> There are also other things we identified as TODOs:
> * Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
> (@Dong Jia, could you take this one)
> * In tree and/or on wiki documentation which is up-to-date and
> more verbose than commit messages are supposed to be (and Dong
> Jia's write-up could be incorporated to). I see this one as
> lower prio though. Any volunteers?

Long-ish things with links etc. should probably go into the wiki.

Anyway, feel free to go ahead (it's a wiki :) We can always change
things later on.

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Halil Pasic]

On 12/04/2017 05:07 PM, Cornelia Huck wrote:
> On Mon, 4 Dec 2017 16:07:27 +0100
> Halil Pasic  wrote:
> 
>> On 12/04/2017 12:15 PM, Cornelia Huck wrote:
>>> On Fri,  1 Dec 2017 15:31:35 +0100
>>> Halil Pasic  wrote:
>>>   
 Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
 to the management software (so it can tell are cssids unrestricted or
 restricted).

 Signed-off-by: Halil Pasic 
 ---

 Boris says having the property on the virtual-css-bridge is good form
 Libvirt PoV. @Shalini: could you verify that things work out fine
 (provided we get at least a preliminary blessing from Connie).

 Consider squashing into "s390x/css: unrestrict cssids".
 ---
  hw/s390x/css-bridge.c | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
 index c4a9735d71..c7e8998680 100644
 --- a/hw/s390x/css-bridge.c
 +++ b/hw/s390x/css-bridge.c
 @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
  DEFINE_PROP_END_OF_LIST(),
  };
  
 +static bool prop_get_true(Object *obj, Error **errp)
 +{
 +return true;
 +}
 +
  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
  {
  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
 *klass, void *data)
  hc->unplug = ccw_device_unplug;
  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
  dc->props = virtual_css_bridge_properties;
 +object_class_property_add_bool(klass, "cssid-unrestricted",
 +   prop_get_true, NULL, NULL);
 +object_class_property_set_description(klass, "cssid-unrestricted",
 +"A css device can use any  cssid, regardless whether virtual" 
  
>>>
>>> extra space -^
>>>   
>>
>> Nod.
>>
 +" or not (read only, always true)",  
>>
>> Do we need "." here ^ ?
> 
> None of the other descs do that.
> 
>>
 +NULL);
  }
  
  static const TypeInfo virtual_css_bridge_info = {  
>>>
>>> Looks reasonable. If this works as expected, I'll squash it into the
>>> previous patch.
>>>   
>>
>> I've just asked Shalini to verify the libvirt perspective.
>>
>> Supposed we verify this works as expected, I read I don't have to spin
>> a v2 and you are going to fix the issues found yourself. Right?
> 
> I'd prefer a v2 that I can simply apply. Let's wait for some more
> acks/r-bs.
> 

Cool with me ;)!

Re: [Qemu-devel] [PATCH 20/25] spapr: add device tree support for the XIVE interrupt mode

[Cédric Le Goater]

On 12/04/2017 08:49 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:50PM +0100, Cédric Le Goater wrote:
>> The XIVE interface for the guest is described in the device tree under
>> the "interrupt-controller" node. A couple of new properties are
>> specific to XIVE :
>>
>>  - "reg"
>>
>>contains the base address and size of the thread interrupt
>>managnement areas (TIMA), also called rings, for the User level and
>>for the Guest OS level. Only the Guest OS level is taken into
>>account today.
>>
>>  - "ibm,xive-eq-sizes"
>>
>>the size of the event queues. One cell per size supported, contains
>>log2 of size, in ascending order.
>>
>>  - "ibm,xive-lisn-ranges"
>>
>>the interrupt numbers ranges assigned to the guest. These are
>>allocated using a simple bitmap.
>>
>> and also under the root node :
>>
>>  - "ibm,plat-res-int-priorities"
>>
>>contains a list of priorities that the hypervisor has reserved for
>>its own use. Simulate ranges as defined by the PowerVM Hypervisor.
>>
>> When the XIVE interrupt mode is activated after the CAS negotiation,
>> the machine will perform a reboot to rebuild the device tree.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive_hcall.c  | 50 
>> +
>>  hw/ppc/spapr.c  |  7 ++-
>>  hw/ppc/spapr_hcall.c|  6 ++
>>  include/hw/ppc/spapr_xive.h |  2 ++
>>  4 files changed, 64 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/spapr_xive_hcall.c b/hw/intc/spapr_xive_hcall.c
>> index 676fe0e2d5c7..60c6c9f4be8f 100644
>> --- a/hw/intc/spapr_xive_hcall.c
>> +++ b/hw/intc/spapr_xive_hcall.c
>> @@ -883,3 +883,53 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
>>  spapr_register_hypercall(H_INT_SYNC, h_int_sync);
>>  spapr_register_hypercall(H_INT_RESET, h_int_reset);
>>  }
>> +
>> +void spapr_xive_populate(sPAPRMachineState *spapr, int nr_servers,
>> + void *fdt, uint32_t phandle)
> 
> Call it spapr_dt_xive() please, I'm trying to standardize on that
> pattern for functions creating DT pieces.

OK. And what about the first argument : sPAPRMachineState *spapr 
or sPAPRXive *xive ? I tend to prefer the first option because
it's related to the interface with the guest, like the hcalls.

> 
>> +{
>> +sPAPRXive *xive = spapr->xive;
>> +int node;
>> +uint64_t timas[2 * 2];
>> +uint32_t lisn_ranges[] = {
>> +cpu_to_be32(0),
>> +cpu_to_be32(nr_servers),
>> +};
>> +uint32_t eq_sizes[] = {
>> +cpu_to_be32(12), /* 4K */
>> +cpu_to_be32(16), /* 64K */
>> +cpu_to_be32(21), /* 2M */
>> +cpu_to_be32(24), /* 16M */
>> +};
>> +uint32_t plat_res_int_priorities[ARRAY_SIZE(reserved_priorities)];
>> +int i;
>> +
>> +for (i = 0; i < ARRAY_SIZE(plat_res_int_priorities); i++) {
>> +plat_res_int_priorities[i] = cpu_to_be32(reserved_priorities[i]);
>> +}
>> +
>> +/* Thread Interrupt Management Areas : User and OS */
>> +for (i = 0; i < 2; i++) {
>> +timas[i * 2] = cpu_to_be64(xive->tm_base + i * (1 << 
>> xive->tm_shift));
>> +timas[i * 2 + 1] = cpu_to_be64(1 << xive->tm_shift);
>> +}
>> +
>> +_FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> 
> You need a unit address here matching the reg property.

Indeed. I didn't notice. Curiously it was taking the first address 
specified in the reg property of the node.

>> +
>> +_FDT(fdt_setprop_string(fdt, node, "name", "interrupt-controller"));
> 
> You don't need to set name properties explicitly for flattened trees.

OK.

Thanks,

C. 



>> +_FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe"));
>> +_FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas)));
>> +
>> +_FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe"));
>> +_FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes,
>> + sizeof(eq_sizes)));
>> +_FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges,
>> + sizeof(lisn_ranges)));
>> +
>> +/* For SLOF */
>> +_FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
>> +_FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
>> +
>> +/* top properties */
>> +_FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
>> + plat_res_int_priorities, 
>> sizeof(plat_res_int_priorities)));
>> +}
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 8b15c0b500d0..3a62369883cc 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1127,7 +1127,12 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>>  _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>>  
>>  /* /interrupt controller */
>> -spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
>> +if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
>> +spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XI

Re: [Qemu-devel] [PATCH 13/25] spapr: introduce the XIVE Event Queues

[Cédric Le Goater]

On 12/04/2017 02:09 AM, David Gibson wrote:
> On Fri, Dec 01, 2017 at 05:36:39PM +0100, Cédric Le Goater wrote:
>> On 12/01/2017 12:35 AM, David Gibson wrote:
>>> On Thu, Nov 30, 2017 at 02:06:27PM +, Cédric Le Goater wrote:
 On 11/30/2017 04:38 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:43PM +0100, Cédric Le Goater wrote:
>> The Event Queue Descriptor (EQD) table, also known as Event Notification
>> Descriptor (END), is one of the internal tables the XIVE interrupt
>> controller uses to redirect exception from event sources to CPU
>> threads.
>>
>> The EQD specifies on which Event Queue the event data should be posted
>> when an exception occurs (later on pulled by the OS) and which server
>> (VPD in XIVE terminology) to notify. The Event Queue is a much more
>> complex structure but we start with a simple model for the sPAPR
>> machine.
>
> Just to clarify my understanding a server / VPD in XIVE would
> typically correspond to a cpu - either real or virtual, yes?

 yes. VP for "virtual processor" and VPD for "virtual processor 
 descriptor" which contains the XIVE interrupt state of the VP 
 when not dispatched. It is still described in some documentation 
 as an NVT : Notification Virtual Target.  

 XIVE concepts were renamed at some time but the old name perdured.
 I am still struggling my way through all the names.


>> There is one XiveEQ per priority and the model chooses to store them
>> under the Xive Interrupt presenter model. It will be retrieved, just
>> like for XICS, through the 'intc' object pointer of the CPU.
>>
>> The EQ indexing follows a simple pattern:
>>
>>(server << 3) | (priority & 0x7)
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive.c| 56 
>> +
>>  hw/intc/xive-internal.h | 50 +++
>>  2 files changed, 106 insertions(+)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 554b25e0884c..983317a6b3f6 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -23,6 +23,7 @@
>>  #include "sysemu/dma.h"
>>  #include "monitor/monitor.h"
>>  #include "hw/ppc/spapr_xive.h"
>> +#include "hw/ppc/spapr.h"
>>  #include "hw/ppc/xics.h"
>>  
>>  #include "xive-internal.h"
>> @@ -34,6 +35,8 @@ struct sPAPRXiveICP {
>>  uint8_t   tima[TM_RING_COUNT * 0x10];
>>  uint8_t   *tima_os;
>>  qemu_irq  output;
>> +
>> +XiveEQeqt[XIVE_PRIORITY_MAX + 1];
>>  };
>>  
>>  static uint64_t spapr_xive_icp_accept(sPAPRXiveICP *icp)
>> @@ -183,6 +186,13 @@ static const MemoryRegionOps spapr_xive_tm_ops = {
>>  },
>>  };
>>  
>> +static sPAPRXiveICP *spapr_xive_icp_get(sPAPRXive *xive, int server)
>> +{
>> +PowerPCCPU *cpu = spapr_find_cpu(server);
>> +
>> +return cpu ? SPAPR_XIVE_ICP(cpu->intc) : NULL;
>> +}
>> +
>>  static void spapr_xive_irq(sPAPRXive *xive, int lisn)
>>  {
>>  
>> @@ -632,6 +642,8 @@ static void spapr_xive_icp_reset(void *dev)
>>  sPAPRXiveICP *xicp = SPAPR_XIVE_ICP(dev);
>>  
>>  memset(xicp->tima, 0, sizeof(xicp->tima));
>> +
>> +memset(xicp->eqt, 0, sizeof(xicp->eqt));
>>  }
>>  
>>  static void spapr_xive_icp_realize(DeviceState *dev, Error **errp)
>> @@ -683,6 +695,23 @@ static void spapr_xive_icp_init(Object *obj)
>>  xicp->tima_os = &xicp->tima[TM_QW1_OS];
>>  }
>>  
>> +static const VMStateDescription vmstate_spapr_xive_icp_eq = {
>> +.name = TYPE_SPAPR_XIVE_ICP "/eq",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.fields = (VMStateField []) {
>> +VMSTATE_UINT32(w0, XiveEQ),
>> +VMSTATE_UINT32(w1, XiveEQ),
>> +VMSTATE_UINT32(w2, XiveEQ),
>> +VMSTATE_UINT32(w3, XiveEQ),
>> +VMSTATE_UINT32(w4, XiveEQ),
>> +VMSTATE_UINT32(w5, XiveEQ),
>> +VMSTATE_UINT32(w6, XiveEQ),
>> +VMSTATE_UINT32(w7, XiveEQ),
>
> Wow.  Super descriptive field names there, but I guess that's not your 
> fault.

 The defines in the "xive-internal.h" give a better view ... 

>> +VMSTATE_END_OF_LIST()
>> +},
>> +};
>> +
>>  static bool vmstate_spapr_xive_icp_needed(void *opaque)
>>  {
>>  /* TODO check machine XIVE support */
>> @@ -696,6 +725,8 @@ static const VMStateDescription 
>> vmstate_spapr_xive_icp = {
>>  .needed = vmstate_spapr_xive_icp_needed,
>>  .fields = (VMStateField[]) {
>>  VMSTATE_BUFFER(tima, sPAPRXiveICP),
>> +VMSTATE_STRUCT_ARRAY(eqt, sPAPRXiveICP, (XIVE_PRIORITY_MAX + 
>> 1), 1,
>>

Re: [Qemu-devel] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_filename()

[Alberto Garcia]

On Mon 20 Nov 2017 09:10:00 PM CET, Max Reitz wrote:
> -static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
> +static void blkdebug_refresh_filename(BlockDriverState *bs)
>  {
>  BDRVBlkdebugState *s = bs->opaque;
> -QDict *opts;
>  const QDictEntry *e;
> -bool force_json = false;
> -
> -for (e = qdict_first(options); e; e = qdict_next(options, e)) {
> -if (strcmp(qdict_entry_key(e), "config") &&
> -strcmp(qdict_entry_key(e), "x-image"))
> -{
> -force_json = true;
> -break;
> -}
> -}
> +int ret;
>  
> -if (force_json && !bs->file->bs->full_open_options) {
> -/* The config file cannot be recreated, so creating a plain filename
> - * is impossible */
> +if (!bs->file->bs->exact_filename[0]) {
>  return;
>  }
>  
> -if (!force_json && bs->file->bs->exact_filename[0]) {
> -int ret = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
> -   "blkdebug:%s:%s", s->config_file ?: "",
> -   bs->file->bs->exact_filename);
> -if (ret >= sizeof(bs->exact_filename)) {
> -/* An overflow makes the filename unusable, so do not report any 
> */
> -bs->exact_filename[0] = 0;
> +for (e = qdict_first(bs->full_open_options); e;
> + e = qdict_next(bs->full_open_options, e))
> +{
> +if (strcmp(qdict_entry_key(e), "config") &&
> +strcmp(qdict_entry_key(e), "image") &&

Shouldn't this be "x-image" ?

Berto

Re: [Qemu-devel] [PATCH v7 for-2.12 22/25] block: Do not copy exact_filename from format file

[Alberto Garcia]

On Mon 20 Nov 2017 09:10:01 PM CET, Max Reitz wrote:
> If the a format BDS's file BDS is in turn a format BDS, we cannot simply
> use the same filename, because when opening a BDS tree based on a
> filename alone, qemu will create only one format node on top of one
> protocol node (disregarding a potential backing file).
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [qemu-s390x] [PATCH RFC 2/2] s390x: attach autogenerated nics

[Cornelia Huck]

On Mon, 4 Dec 2017 12:17:06 +0100
Christian Borntraeger  wrote:

> On 11/28/2017 02:46 PM, Cornelia Huck wrote:
> > The autogenerated nics should be treated as any other device; use
> > qdev_set_id() to have them show up under peripheral-anon.
> >   
> I think this is fine, but then I ask myself how x86 does this. So I tried to 
> find out how the pc-q35 machine does this but I somehow failed to understand
> how they do it. Do you have any clue?

It seems they don't. If you start up a machine with only autogenerated
devices, you won't find anything under peripheral{-anon}, but several
devices under unattached.

So, maybe we should change this for everything? Or just leave it alone?

(The css-bridge change is a different thing IMO, it clearly should be
attached to the machine.)

> 
> > Signed-off-by: Cornelia Huck 
> > ---
> >  hw/s390x/s390-virtio-ccw.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index a23b8aec9f..830bae9d0f 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -35,6 +35,7 @@
> >  #include "cpu_models.h"
> >  #include "qapi/qmp/qerror.h"
> >  #include "hw/nmi.h"
> > +#include "include/monitor/qdev.h"
> > 
> >  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> >  {
> > @@ -259,6 +260,7 @@ static void s390_create_virtio_net(BusState *bus, const 
> > char *name)
> > 
> >  dev = qdev_create(bus, name);
> >  qdev_set_nic_properties(dev, nd);
> > +qdev_set_id(dev, NULL);
> >  qdev_init_nofail(dev);
> >  }
> >  }
> >   
> 

Re: [Qemu-devel] [PATCH v7 for-2.12 25/25] block/null: Generate filename even with latency-ns

[Alberto Garcia]

On Mon 20 Nov 2017 09:10:04 PM CET, Max Reitz wrote:
> While we cannot represent the latency-ns option in a filename, it is not
> a significant option so not being able to should not stop us from
> generating a filename nonetheless.
>
> Signed-off-by: Max Reitz 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH RFC 0/2] s390x: cut down on unattached devices

[Cornelia Huck]

On Mon, 4 Dec 2017 15:47:37 +0100
Halil Pasic  wrote:

> On 12/04/2017 10:22 AM, Cornelia Huck wrote:
> > On Fri, 1 Dec 2017 15:41:21 +0100
> > Halil Pasic  wrote:
> >   
> >> On 11/28/2017 04:21 PM, Halil Pasic wrote:
> >> [..]  
> > Otherwise at first glance both patches seem sane.
> 
>  Can I count this as an ack, or do you plan to do more review?
> 
> >>>
> >>> Yes I was planning to give it another look. And I do already
> >>> have questions. Isn't the QOM composition tree API? I mean
> >>> let's assume the QMP commands working on this tree are not completely
> >>> useless. How is client code (management software) supposed to work,
> >>> assumed it can rely on paths of e.g. properties being stable. Just
> >>> imagine we had this default-cssid property (for the sake of the
> >>> argument, not like we want it) on the css bridge.
> >>
> >> Ping! I would like to get this clarified before proceeding with reviewing
> >> this series.  
> > 
> > [It might be helpful to not drop cc:s.]
> >   
> 
> Sorry. Wrong button.
> 
> > I don't think we really want a static tree. As long as the devices are
> > locateable, it should be fine.
> >   
> 
> What do you mean by locateable in particular?
> 
> Let's say I'm management software guy who want's to access a certain
> property of a certain device. For that I'm supposed to use qom-get. Now
> qom-get takes a path as input argument (absolute or relative). The question
> is, how I'm supposed to figure out this path as management software developer?
> In other words what is the algorithm?

I'd expect qom-tree to put out a path to the right object.

No idea if/how much management software relies on this. But hardcoded
paths sound fragile to me.

> 
> One naive approach would be, to assume that the path is stable between
> releases. So I have to figure it out when I'm implementing the stuff,
> and then it keeps working ever after. I read your answer as this naive
> approach is wrong.
> 
> (BTW I find static also confusing in this context.)
> 
> [..]
> 

Re: [Qemu-devel] [PATCH v7 for-2.12 24/25] block/curl: Implement bdrv_refresh_filename()

[Alberto Garcia]

On Mon 20 Nov 2017 09:10:03 PM CET, Max Reitz wrote:
> Signed-off-by: Max Reitz 
> ---
>  block/curl.c | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/block/curl.c b/block/curl.c
> index 11318a9a29..fe57223fda 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -957,6 +957,20 @@ static int64_t curl_getlength(BlockDriverState *bs)
>  return s->len;
>  }
>  
> +static void curl_refresh_filename(BlockDriverState *bs)
> +{
> +BDRVCURLState *s = bs->opaque;
> +
> +if (!s->sslverify || s->cookie ||
> +s->username || s->password || s->proxyusername || s->proxypassword)
> +{

Is !s->sslverify negative because that setting is true by default?

Berto

Re: [Qemu-devel] [PATCH v7 for-2.12 20/25] block: Generically refresh runtime options

[Alberto Garcia]

On Mon 20 Nov 2017 09:09:59 PM CET, Max Reitz wrote:
> Instead of having every block driver which implements
> bdrv_refresh_filename() copy all of the significant runtime options over
> to bs->full_open_options, implement this process generically in
> bdrv_refresh_filename().
>
> This patch only adds this new generic implementation, it does not remove
> the old functionality. This is done in a follow-up patch.
>
> With this patch, some superfluous information (that should never have
> been there) may be removed from some JSON filenames, as can be seen in
> the change to iotest 110's reference output.  In case of 191, backing
> nodes that have not been overridden are now removed from the filename.
>
> Signed-off-by: Max Reitz 


> +static const char *const *significant_options(BlockDriverState *bs,
> +  const char *const *curopt)
> +{
> +static const char *const global_options[] = {
> +"driver", "filename", "base-directory", NULL
> +};

You forgot to remove "base-directory" here from v6, but else the patch
looks good.

Reviewed-by: Alberto Garcia 

Berto