Re: [Qemu-devel] [PATCH v10 06/13] hw/intc: RX62N interrupt controller (ICUa)

2019-05-12 Thread Yoshinori Sato
On Thu, 09 May 2019 01:27:40 +0900,
Philippe Mathieu-Daudé wrote:
> 
> On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> > This implementation supported only ICUa.
> > Hardware manual.
> > https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> > 
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/intc/rx_icu.h |  49 +++
> >  hw/intc/rx_icu.c | 375 
> > +++
> >  hw/intc/Makefile.objs|   1 +
> >  3 files changed, 425 insertions(+)
> >  create mode 100644 include/hw/intc/rx_icu.h
> >  create mode 100644 hw/intc/rx_icu.c
> > 
> > diff --git a/include/hw/intc/rx_icu.h b/include/hw/intc/rx_icu.h
> > new file mode 100644
> > index 00..bc46b3079b
> > --- /dev/null
> > +++ b/include/hw/intc/rx_icu.h
> > @@ -0,0 +1,49 @@
> > +#ifndef RX_ICU_H
> > +#define RX_ICU_H
> > +
> > +#include "qemu-common.h"
> > +#include "hw/irq.h"
> > +
> > +struct IRQSource {
> > +int sense;
> 
> Hmm can you use the enum?
> 
> > +int level;
> > +};
> > +
> > +struct RXICUState {
> > +SysBusDevice parent_obj;
> > +
> > +MemoryRegion memory;
> > +struct IRQSource src[256];
> > +char *icutype;
> > +uint32_t nr_irqs;
> > +uint32_t *map;
> > +uint32_t nr_sense;
> > +uint32_t *init_sense;
> > +
> > +uint8_t ir[256];
> > +uint8_t dtcer[256];
> > +uint8_t ier[32];
> > +uint8_t ipr[142];
> > +uint8_t dmasr[4];
> > +uint16_t fir;
> > +uint8_t nmisr;
> > +uint8_t nmier;
> > +uint8_t nmiclr;
> > +uint8_t nmicr;
> > +int req_irq;
> > +qemu_irq _irq;
> > +qemu_irq _fir;
> > +qemu_irq _swi;
> > +};
> > +typedef struct RXICUState RXICUState;
> > +
> > +#define TYPE_RXICU "rxicu"
> > +#define RXICU(obj) OBJECT_CHECK(RXICUState, (obj), TYPE_RXICU)
> > +
> > +#define SWI 27
> 
> enum {
> 
> > +#define TRG_LEVEL 0
> > +#define TRG_NEDGE 1
> > +#define TRG_PEDGE 2
> > +#define TRG_BEDGE 3
> 
> };
> 
> > +
> > +#endif /* RX_ICU_H */
> > diff --git a/hw/intc/rx_icu.c b/hw/intc/rx_icu.c
> > new file mode 100644
> > index 00..345e047a45
> > --- /dev/null
> > +++ b/hw/intc/rx_icu.c
> > @@ -0,0 +1,375 @@
> > +/*
> > + * RX Interrupt control unit
> 
> "Interrupt Control Unit"
> 
> "Warning: Only ICUa is supported."
> 
> > + *
> > + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> > + * (Rev.1.40 R01UH0033EJ0140)
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope 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 .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/intc/rx_icu.h"
> > +#include "qemu/error-report.h"
> > +
> > +REG8(IR, 0)
> > +  FIELD(IR, IR,  0, 1)
> > +REG8(DTCER, 0x100)
> > +  FIELD(DTCER, DTCE,  0, 1)
> > +REG8(IER, 0x200)
> > +REG8(SWINTR, 0x2e0)
> > +  FIELD(SWINTR, SWINT, 0, 1)
> > +REG16(FIR, 0x2f0)
> > +  FIELD(FIR, FVCT, 0, 8)
> > +  FIELD(FIR, FIEN, 15, 1)
> > +REG8(IPR, 0x300)
> > +  FIELD(IPR, IPR, 0, 4)
> > +REG8(DMRSR, 0x400)
> > +REG8(IRQCR, 0x500)
> > +  FIELD(IRQCR, IRQMD, 2, 2)
> > +REG8(NMISR, 0x580)
> > +  FIELD(NMISR, NMIST, 0, 1)
> > +  FIELD(NMISR, LVDST, 1, 1)
> > +  FIELD(NMISR, OSTST, 2, 1)
> > +REG8(NMIER, 0x581)
> > +  FIELD(NMIER, NMIEN, 0, 1)
> > +  FIELD(NMIER, LVDEN, 1, 1)
> > +  FIELD(NMIER, OSTEN, 2, 1)
> > +REG8(NMICLR, 0x582)
> > +  FIELD(NMICLR, NMICLR, 0, 1)
> > +  FIELD(NMICLR, OSTCLR, 2, 1)
> > +REG8(NMICR, 0x583)
> > +  FIELD(NMICR, NMIMD, 3, 1)
> > +
> > +#define request(icu, n) (icu->ipr[icu->map[n]] << 8 | n)
> > +
> > +static qemu_irq *rxicu_pin(RXICUState *icu, int n_IRQ)
> > +{
> > +if ((icu->fir & R_FIR_FIEN_MASK) &&
> > +(icu->fir & R_FIR_FVCT_MASK) == n_IRQ) {
> > +return >_fir;
> > +} else {
> > +return >_irq;
> > +}
> > +}
> > +
> > +static void rxicu_request(RXICUState *icu, int n_IRQ)
> > +{
> > +int enable;
> > +
> > +enable = icu->ier[n_IRQ / 8] & (1 << (n_IRQ & 7));
> > +if (n_IRQ > 0 && enable != 0 && atomic_read(>req_irq) < 0) {
> > +atomic_set(>req_irq, n_IRQ);
> > +qemu_set_irq(*rxicu_pin(icu, n_IRQ), request(icu, n_IRQ));
> > +}
> > +}
> > +
> > +static void rxicu_set_irq(void *opaque, int 

Re: [Qemu-devel] [qemu-s390x] [PATCH v4] s390: diagnose 318 info reset and migration support

2019-05-12 Thread Thomas Huth
On 09/05/2019 22.50, Collin Walling wrote:
> On 5/9/19 5:58 AM, Christian Borntraeger wrote:
>>
>>
>> On 02.05.19 00:31, Collin Walling wrote:
>>> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
>>> be intercepted by SIE and handled via KVM. Let's introduce some
>>> functions to communicate between QEMU and KVM via ioctls. These
>>> will be used to get/set the diag318 related information (also known
>>> as the "Control Program Code" or "CPC"), as well as check the system
>>> if KVM supports handling this instruction.
>>>
>>> The availability of this instruction is determined by byte 134, bit 0
>>> of the Read Info block. This coincidentally expands into the space used
>>> for CPU entries, which means VMs running with the diag318 capability
>>> will have a reduced maximum CPU count. To alleviate this, let's
>>> calculate
>>> the actual max CPU entry space by subtracting the size of Read Info from
>>> the SCCB size then dividing that number by the size of a CPU entry. If
>>> this value is less than the value denoted by S390_MAX_CPUS, then let's
>>> reduce the max cpus for s390 from 248 to 240 in an effort to anticipate
>>> this potentially happening again in the future.
>>>
>>> In order to simplify the migration and system reset requirements of
>>> the diag318 data, let's introduce it as a device class and include
>>> a VMStateDescription.
>>>
>>> Diag318 is reset on during modified clear and load normal.
>>>
>>> Signed-off-by: Collin Walling 
>>> ---
>>>   hw/s390x/Makefile.objs   |   1 +
>>>   hw/s390x/diag318.c   | 100
>>> +++
>>>   hw/s390x/diag318.h   |  39 +
>>>   hw/s390x/s390-virtio-ccw.c   |  23 ++
>>>   hw/s390x/sclp.c  |   5 +++
>>>   include/hw/s390x/sclp.h  |   2 +
>>>   linux-headers/asm-s390/kvm.h |   4 ++
>>>   target/s390x/kvm-stub.c  |  15 +++
>>>   target/s390x/kvm.c   |  32 ++
>>>   target/s390x/kvm_s390x.h |   3 ++
>>>   10 files changed, 224 insertions(+)
>>>   create mode 100644 hw/s390x/diag318.c
>>>   create mode 100644 hw/s390x/diag318.h
>>>
>>> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
>>> index e02ed80..93621dc 100644
>>> --- a/hw/s390x/Makefile.objs
>>> +++ b/hw/s390x/Makefile.objs
>>> @@ -34,3 +34,4 @@ obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
>>>   obj-y += s390-ccw.o
>>>   obj-y += ap-device.o
>>>   obj-y += ap-bridge.o
>>> +obj-y += diag318.o
>>> diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
>>> new file mode 100644
>>> index 000..94b44da
>>> --- /dev/null
>>> +++ b/hw/s390x/diag318.c
>>> @@ -0,0 +1,100 @@
>>> +/*
>>> + * DIAGNOSE 0x318 functions for reset and migration
>>> + *
>>> + * Copyright IBM, Corp. 2019
>>> + *
>>> + * Authors:
>>> + *  Collin Walling 
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2
>>> or (at your
>>> + * option) any later version. See the COPYING file in the top-level
>>> directory.
>>> + */
>>> +
>>> +#include "hw/s390x/diag318.h"
>>> +#include "qapi/error.h"
>>> +#include "kvm_s390x.h"
>>> +#include "sysemu/kvm.h"
>>> +
>>> +static int diag318_post_load(void *opaque, int version_id)
>>> +{
>>> +    DIAG318State *d = opaque;
>>> +
>>> +    kvm_s390_set_cpc(d->cpc);
>>> +
>>> +    /* It is not necessary to retain a copy of the cpc after
>>> migration. */
>>> +    d->cpc = 0;
>>
>> But we also do not need to zero it out? Can't you just drop these 2
>> lines?
>>
>>
> 
> Absolutely. I'll drop them.
> 
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int diag318_pre_save(void *opaque)
>>> +{
>>> +    DIAG318State *d = opaque;
>>> +
>>> +    kvm_s390_get_cpc(>cpc);
>>> +    return 0;
>>> +}
>>> +
>>> +static bool diag318_needed(void *opaque)
>>> +{
>>> +    DIAG318State *d = opaque;
>>> +
>>> +    return d->enabled;
>>> +}
>>
>> I would like to have a cpumodel entry that allows to disable that
>> feature. And we should
>> then check for this.
>>
> 
> Noted.
> 
>>> +
>>> +const VMStateDescription vmstate_diag318 = {
>>> +    .name = "vmstate_diag318",
>>> +    .post_load = diag318_post_load,
>>> +    .pre_save = diag318_pre_save,
>>> +    .version_id = 1,
>>> +    .minimum_version_id = 1,
>>> +    .needed = diag318_needed,
>>> +    .fields = (VMStateField[]) {
>>> +    VMSTATE_UINT64(cpc, DIAG318State),
>>> +    VMSTATE_END_OF_LIST()
>>> +    }
>>> +};
>>> +
>>> +static void s390_diag318_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    DIAG318State *d = DIAG318(dev);
>>> +
>>> +    if (kvm_s390_has_diag318()) {
>>> +    d->enabled = true;
>>> +    }
>>
>> same here -> cpumodel
>> Then we can get rid of the enabled bool.
> 
> Noted.
> 
> [...]
>>>     static void s390_cpu_plug(HotplugHandler *hotplug_dev,
>>> @@ -570,6 +585,7 @@ static void machine_set_loadparm(Object *obj,
>>> const char *val, Error **errp)
>>>   ms->loadparm[i] = ' '; /* pad right with spaces */
>>>   }
>>>   }
>>> +
>>>   static inline void 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v8 3/6] target/ppc: Handle NMI guest exit

2019-05-12 Thread David Gibson
On Mon, May 13, 2019 at 11:10:28AM +0530, Aravinda Prasad wrote:
> 
> 
> On Friday 10 May 2019 09:55 PM, Greg Kurz wrote:
> > On Mon, 22 Apr 2019 12:33:16 +0530
> > Aravinda Prasad  wrote:
> > 
> >> Memory error such as bit flips that cannot be corrected
> >> by hardware are passed on to the kernel for handling.
> >> If the memory address in error belongs to guest then
> >> the guest kernel is responsible for taking suitable action.
> >> Patch [1] enhances KVM to exit guest with exit reason
> >> set to KVM_EXIT_NMI in such cases. This patch handles
> >> KVM_EXIT_NMI exit.
> >>
> >> [1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
> >> (e20bbd3d and related commits)
> >>
> >> Signed-off-by: Aravinda Prasad 
> >> ---
> >>  hw/ppc/spapr.c  |3 +++
> >>  hw/ppc/spapr_events.c   |   22 ++
> >>  hw/ppc/spapr_rtas.c |5 +
> >>  include/hw/ppc/spapr.h  |6 ++
> >>  target/ppc/kvm.c|   16 
> >>  target/ppc/kvm_ppc.h|2 ++
> >>  target/ppc/trace-events |2 ++
> >>  7 files changed, 56 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 6642cb5..2779efe 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1806,6 +1806,7 @@ static void spapr_machine_reset(void)
> >>  
> >>  spapr->cas_reboot = false;
> >>  
> >> +spapr->mc_status = -1;
> >>  spapr->guest_machine_check_addr = -1;
> >>  
> >>  /* Signal all vCPUs waiting on this condition */
> >> @@ -2106,6 +2107,7 @@ static const VMStateDescription 
> >> vmstate_spapr_machine_check = {
> >>  .minimum_version_id = 1,
> >>  .fields = (VMStateField[]) {
> >>  VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
> >> +VMSTATE_INT32(mc_status, SpaprMachineState),
> >>  VMSTATE_END_OF_LIST()
> >>  },
> >>  };
> >> @@ -3085,6 +3087,7 @@ static void spapr_machine_init(MachineState *machine)
> >>  kvmppc_spapr_enable_inkernel_multitce();
> >>  }
> >>  
> >> +spapr->mc_status = -1;
> > 
> > Since this is done at reset, do we need it here ?
> 
> Yes, because we need to initialize this on a fresh boot. I need to
> check, but if spapr_machine_reset() is called every time a system boots

It is.

> then we don't need qemu_cond_init() here as well.
> 
> > 
> >>  qemu_cond_init(>mc_delivery_cond);
> >>  }
> >>  
> >> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> >> index ae0f093..9922a23 100644
> >> --- a/hw/ppc/spapr_events.c
> >> +++ b/hw/ppc/spapr_events.c
> >> @@ -620,6 +620,28 @@ void 
> >> spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
> >>  RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, 
> >> _id);
> >>  }
> >>  
> >> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
> >> +{
> >> +SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >> +
> >> +while (spapr->mc_status != -1) {
> >> +/*
> >> + * Check whether the same CPU got machine check error
> >> + * while still handling the mc error (i.e., before
> >> + * that CPU called "ibm,nmi-interlock"
> > 
> > Missing )
> 
> ok.
> 
> > 
> >> + */
> >> +if (spapr->mc_status == cpu->vcpu_id) {
> >> +qemu_system_guest_panicked(NULL);
> > 
> > If we don't also return, is there a chance we end up stuck in
> > qemu_cond_wait_iothread() below ?
> 
> I think I need to return here
> 
> 
> > 
> >> +}
> >> +qemu_cond_wait_iothread(>mc_delivery_cond);
> >> +/* Meanwhile if the system is reset, then just return */
> >> +if (spapr->guest_machine_check_addr == -1) {
> >> +return;
> >> +}
> >> +}
> >> +spapr->mc_status = cpu->vcpu_id;
> >> +}
> >> +
> >>  static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
> >>  uint32_t token, uint32_t nargs,
> >>  target_ulong args,
> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >> index c2f3991..d3499f9 100644
> >> --- a/hw/ppc/spapr_rtas.c
> >> +++ b/hw/ppc/spapr_rtas.c
> >> @@ -375,6 +375,11 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> >>  /* NMI register not called */
> >>  rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> >>  } else {
> >> +/*
> >> + * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> >> + * hence unset mc_status.
> >> + */
> >> +spapr->mc_status = -1;
> >>  qemu_cond_signal(>mc_delivery_cond);
> >>  rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >>  }
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index ec6f33e..f7204d0 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -189,6 +189,11 @@ struct SpaprMachineState {
> >>  
> >>  /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls 
> >> */
> >>  target_ulong 

Re: [Qemu-devel] [PATCH v2] target/ppc: Fix xvabs[sd]p, xvnabs[sd]p, xvneg[sd]p, xvcpsgn[sd]p

2019-05-12 Thread David Gibson
On Fri, May 10, 2019 at 04:02:56PM +0100, Mark Cave-Ayland wrote:
> On 09/05/2019 01:49, Anton Blanchard wrote:
> 
> > We were using set_cpu_vsr*() when we should have used get_cpu_vsr*().
> > 
> > Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() 
> > helpers for VSR register access")
> > Signed-off-by: Anton Blanchard 
> > ---
> >  target/ppc/translate/vsx-impl.inc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/translate/vsx-impl.inc.c 
> > b/target/ppc/translate/vsx-impl.inc.c
> > index b487136d52..4b7627f53b 100644
> > --- a/target/ppc/translate/vsx-impl.inc.c
> > +++ b/target/ppc/translate/vsx-impl.inc.c
> > @@ -859,8 +859,8 @@ static void glue(gen_, name)(DisasContext *ctx) 
> >  \
> >  xbh = tcg_temp_new_i64();\
> >  xbl = tcg_temp_new_i64();\
> >  sgm = tcg_temp_new_i64();\
> > -set_cpu_vsrh(xB(ctx->opcode), xbh);  \
> > -set_cpu_vsrl(xB(ctx->opcode), xbl);  \
> > +get_cpu_vsrh(xbh, xB(ctx->opcode));  \
> > +get_cpu_vsrl(xbl, xB(ctx->opcode));  \
> >  tcg_gen_movi_i64(sgm, sgn_mask); \
> >  switch (op) {\
> >  case OP_ABS: {   \
> 
> Reviewed-by: Mark Cave-Ayland 

Applied, thanks.

> 
> 
> ATB,
> 
> Mark.
> 

-- 
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 v10 08/13] hw/char: RX62N serial communication interface (SCI)

2019-05-12 Thread Yoshinori Sato
On Thu, 09 May 2019 01:16:58 +0900,
Philippe Mathieu-Daudé wrote:
> 
> On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> > This module supported only non FIFO type.
> > Hardware manual.
> > https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> > 
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/char/renesas_sci.h |  45 ++
> >  hw/char/renesas_sci.c | 340 
> > ++
> >  hw/char/Kconfig   |   3 +
> >  hw/char/Makefile.objs |   2 +-
> >  4 files changed, 389 insertions(+), 1 deletion(-)
> >  create mode 100644 include/hw/char/renesas_sci.h
> >  create mode 100644 hw/char/renesas_sci.c
> > 
> > diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
> > new file mode 100644
> > index 00..50d1336944
> > --- /dev/null
> > +++ b/include/hw/char/renesas_sci.h
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Renesas Serial Communication Interface
> > + *
> > + * Copyright (c) 2018 Yoshinori Sato
> > + *
> > + * This code is licensed under the GPL version 2 or later.
> > + *
> > + */
> > +
> > +#include "chardev/char-fe.h"
> > +#include "qemu/timer.h"
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_RENESAS_SCI "renesas-sci"
> > +#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
> > +
> > +enum {
> > +ERI = 0,
> > +RXI = 1,
> > +TXI = 2,
> > +TEI = 3,
> > +SCI_NR_IRQ = 4,
> > +};
> > +
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion memory;
> > +
> > +uint8_t smr;
> > +uint8_t brr;
> > +uint8_t scr;
> > +uint8_t tdr;
> > +uint8_t ssr;
> > +uint8_t rdr;
> > +uint8_t scmr;
> > +uint8_t semr;
> > +
> > +uint8_t read_ssr;
> > +int64_t trtime;
> > +int64_t rx_next;
> > +QEMUTimer *timer;
> > +CharBackend chr;
> > +uint64_t input_freq;
> > +qemu_irq irq[SCI_NR_IRQ];
> > +} RSCIState;
> > diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> > new file mode 100644
> > index 00..2e7c3e460e
> > --- /dev/null
> > +++ b/hw/char/renesas_sci.c
> > @@ -0,0 +1,340 @@
> > +/*
> > + * Renesas Serial Communication Interface
> > + *
> > + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> > + * (Rev.1.40 R01UH0033EJ0140)
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope 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 .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "qemu-common.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/char/renesas_sci.h"
> > +#include "qemu/error-report.h"
> > +
> > +/* SCI register map */
> > +REG8(SMR, 0)
> > +  FIELD(SMR, CKS,  0, 2)
> > +  FIELD(SMR, MP,   2, 1)
> > +  FIELD(SMR, STOP, 3, 1)
> > +  FIELD(SMR, PM,   4, 1)
> > +  FIELD(SMR, PE,   5, 1)
> > +  FIELD(SMR, CHR,  6, 1)
> > +  FIELD(SMR, CM,   7, 1)
> > +REG8(BRR, 1)
> > +REG8(SCR, 2)
> > +  FIELD(SCR, CKE, 0, 2)
> > +  FIELD(SCR, TEIE, 2, 1)
> > +  FIELD(SCR, MPIE, 3, 1)
> > +  FIELD(SCR, RE,   4, 1)
> > +  FIELD(SCR, TE,   5, 1)
> > +  FIELD(SCR, RIE,  6, 1)
> > +  FIELD(SCR, TIE,  7, 1)
> > +REG8(TDR, 3)
> > +REG8(SSR, 4)
> > +  FIELD(SSR, MPBT, 0, 1)
> > +  FIELD(SSR, MPB,  1, 1)
> > +  FIELD(SSR, TEND, 2, 1)
> > +  FIELD(SSR, ERR, 3, 3)
> > +FIELD(SSR, PER,  3, 1)
> > +FIELD(SSR, FER,  4, 1)
> > +FIELD(SSR, ORER, 5, 1)
> > +  FIELD(SSR, RDRF, 6, 1)
> > +  FIELD(SSR, TDRE, 7, 1)
> > +REG8(RDR, 5)
> > +REG8(SCMR, 6)
> > +  FIELD(SCMR, SMIF, 0, 1)
> > +  FIELD(SCMR, SINV, 2, 1)
> > +  FIELD(SCMR, SDIR, 3, 1)
> > +  FIELD(SCMR, BCP2, 7, 1)
> > +REG8(SEMR, 7)
> > +  FIELD(SEMR, ACS0, 0, 1)
> > +  FIELD(SEMR, ABCS, 4, 1)
> > +
> > +static int can_receive(void *opaque)
> > +{
> > +RSCIState *sci = RSCI(opaque);
> > +if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> > +return 0;
> > +} else {
> > +return FIELD_EX8(sci->scr, SCR, RE);
> > +}
> > +}
> > +
> > +static void receive(void *opaque, const uint8_t *buf, int size)
> > +{
> > +RSCIState *sci = RSCI(opaque);
> > +sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
> > +if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
> > +sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v8 3/6] target/ppc: Handle NMI guest exit

2019-05-12 Thread Aravinda Prasad



On Friday 10 May 2019 09:55 PM, Greg Kurz wrote:
> On Mon, 22 Apr 2019 12:33:16 +0530
> Aravinda Prasad  wrote:
> 
>> Memory error such as bit flips that cannot be corrected
>> by hardware are passed on to the kernel for handling.
>> If the memory address in error belongs to guest then
>> the guest kernel is responsible for taking suitable action.
>> Patch [1] enhances KVM to exit guest with exit reason
>> set to KVM_EXIT_NMI in such cases. This patch handles
>> KVM_EXIT_NMI exit.
>>
>> [1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
>> (e20bbd3d and related commits)
>>
>> Signed-off-by: Aravinda Prasad 
>> ---
>>  hw/ppc/spapr.c  |3 +++
>>  hw/ppc/spapr_events.c   |   22 ++
>>  hw/ppc/spapr_rtas.c |5 +
>>  include/hw/ppc/spapr.h  |6 ++
>>  target/ppc/kvm.c|   16 
>>  target/ppc/kvm_ppc.h|2 ++
>>  target/ppc/trace-events |2 ++
>>  7 files changed, 56 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 6642cb5..2779efe 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1806,6 +1806,7 @@ static void spapr_machine_reset(void)
>>  
>>  spapr->cas_reboot = false;
>>  
>> +spapr->mc_status = -1;
>>  spapr->guest_machine_check_addr = -1;
>>  
>>  /* Signal all vCPUs waiting on this condition */
>> @@ -2106,6 +2107,7 @@ static const VMStateDescription 
>> vmstate_spapr_machine_check = {
>>  .minimum_version_id = 1,
>>  .fields = (VMStateField[]) {
>>  VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
>> +VMSTATE_INT32(mc_status, SpaprMachineState),
>>  VMSTATE_END_OF_LIST()
>>  },
>>  };
>> @@ -3085,6 +3087,7 @@ static void spapr_machine_init(MachineState *machine)
>>  kvmppc_spapr_enable_inkernel_multitce();
>>  }
>>  
>> +spapr->mc_status = -1;
> 
> Since this is done at reset, do we need it here ?

Yes, because we need to initialize this on a fresh boot. I need to
check, but if spapr_machine_reset() is called every time a system boots
then we don't need qemu_cond_init() here as well.

> 
>>  qemu_cond_init(>mc_delivery_cond);
>>  }
>>  
>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>> index ae0f093..9922a23 100644
>> --- a/hw/ppc/spapr_events.c
>> +++ b/hw/ppc/spapr_events.c
>> @@ -620,6 +620,28 @@ void 
>> spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
>>  RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, 
>> _id);
>>  }
>>  
>> +void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>> +{
>> +SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> +
>> +while (spapr->mc_status != -1) {
>> +/*
>> + * Check whether the same CPU got machine check error
>> + * while still handling the mc error (i.e., before
>> + * that CPU called "ibm,nmi-interlock"
> 
> Missing )

ok.

> 
>> + */
>> +if (spapr->mc_status == cpu->vcpu_id) {
>> +qemu_system_guest_panicked(NULL);
> 
> If we don't also return, is there a chance we end up stuck in
> qemu_cond_wait_iothread() below ?

I think I need to return here


> 
>> +}
>> +qemu_cond_wait_iothread(>mc_delivery_cond);
>> +/* Meanwhile if the system is reset, then just return */
>> +if (spapr->guest_machine_check_addr == -1) {
>> +return;
>> +}
>> +}
>> +spapr->mc_status = cpu->vcpu_id;
>> +}
>> +
>>  static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
>>  uint32_t token, uint32_t nargs,
>>  target_ulong args,
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index c2f3991..d3499f9 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -375,6 +375,11 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>>  /* NMI register not called */
>>  rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>  } else {
>> +/*
>> + * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
>> + * hence unset mc_status.
>> + */
>> +spapr->mc_status = -1;
>>  qemu_cond_signal(>mc_delivery_cond);
>>  rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>  }
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index ec6f33e..f7204d0 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -189,6 +189,11 @@ struct SpaprMachineState {
>>  
>>  /* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
>>  target_ulong guest_machine_check_addr;
>> +/*
>> + * mc_status is set to -1 if mc is not in progress, else is set to the 
>> CPU
>> + * handling the mc.
>> + */
>> +int mc_status;
>>  QemuCond mc_delivery_cond;
>>  
>>  /*< public >*/
>> @@ -792,6 +797,7 @@ void spapr_clear_pending_events(SpaprMachineState 
>> *spapr);
>>  int 

Re: [Qemu-devel] [PATCH v8 4/6] target/ppc: Build rtas error log upon an MCE

2019-05-12 Thread Aravinda Prasad



On Friday 10 May 2019 03:22 PM, David Gibson wrote:
> On Fri, May 10, 2019 at 12:35:13PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Friday 10 May 2019 12:12 PM, David Gibson wrote:
>>> On Mon, Apr 22, 2019 at 12:33:26PM +0530, Aravinda Prasad wrote:

[...]

 +/* Save gpr[3] in the guest endian mode */
 +if ((*pcc->interrupts_big_endian)(cpu)) {
 +env->gpr[3] = cpu_to_be64(rtas_addr + RTAS_ERRLOG_OFFSET);
>>>
>>> I don't think this is right.  AIUI env->gpr[] are all stored in *host*
>>> endianness (for ease of doing arithmetic).
>>
>> env-gpr[3] is later used by guest to fetch the RTAS log. My guess is
>> that we will not do an endianness change of all the gprs during a switch
>> from host to guest (that will be costly).
> 
> There's no need to "change endianness".  In TCG the host needs to do
> arithmetic on the values and so they are in host endian.  With KVM the
> env values are only synchronized when we enter/exit KVM and they're
> going to registers, not memory and so have no endianness.

Ah.. ok.

> 
>> But let me cross check.
>>
>>>
 +} else {
 +env->gpr[3] = cpu_to_le64(rtas_addr + RTAS_ERRLOG_OFFSET);
 +}
 +
 +env->nip = spapr->guest_machine_check_addr;
 +}
 +
  void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
  {
  SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 @@ -640,6 +881,10 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool 
 recovered)
  }
  }
  spapr->mc_status = cpu->vcpu_id;
 +
 +spapr_mce_dispatch_elog(cpu, recovered);
 +
 +return;
  }
  
  static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
 diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
 index f7204d0..03f34bf 100644
 --- a/include/hw/ppc/spapr.h
 +++ b/include/hw/ppc/spapr.h
 @@ -661,6 +661,9 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, 
 target_ulong opcode,
  #define DIAGNOSTICS_RUN_MODE_IMMEDIATE 2
  #define DIAGNOSTICS_RUN_MODE_PERIODIC  3
  
 +/* Offset from rtas-base where error log is placed */
 +#define RTAS_ERRLOG_OFFSET   0x25
>>>
>>> Is this offset PAPR defined, or chosen here?  Using an entirely
>>> unaliged (odd) address seems a very strange choice.
>>
>> This is not PAPR defined. I will make it 0x30. Or do you prefer any
>> other offset?
> 
> 0x30 should be fine.

ok..

> 

-- 
Regards,
Aravinda




Re: [Qemu-devel] [Qemu-ppc] [PATCH v8 1/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

2019-05-12 Thread Aravinda Prasad



On Friday 10 May 2019 08:03 PM, Greg Kurz wrote:
> On Fri, 10 May 2019 11:06:04 +0200
> Greg Kurz  wrote:
> 
>> On Mon, 22 Apr 2019 12:32:58 +0530
>> Aravinda Prasad  wrote:
>>
>>> This patch adds support in QEMU to handle "ibm,nmi-register"
>>> and "ibm,nmi-interlock" RTAS calls.
>>>
>>> The machine check notification address is saved when the
>>> OS issues "ibm,nmi-register" RTAS call.
>>>
>>> This patch also handles the case when multiple processors
>>> experience machine check at or about the same time by
>>> handling "ibm,nmi-interlock" call. In such cases, as per
>>> PAPR, subsequent processors serialize waiting for the first
>>> processor to issue the "ibm,nmi-interlock" call. The second
>>> processor that also received a machine check error waits
>>> till the first processor is done reading the error log.
>>> The first processor issues "ibm,nmi-interlock" call
>>> when the error log is consumed. This patch implements the
>>> releasing part of the error-log while subsequent patch
>>> (which builds error log) handles the locking part.
>>>
>>> Signed-off-by: Aravinda Prasad 
>>> ---
>>>  hw/ppc/spapr.c |   18 ++
>>>  hw/ppc/spapr_rtas.c|   61 
>>> 
>>>  include/hw/ppc/spapr.h |9 ++-
>>>  3 files changed, 87 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index c56939a..6642cb5 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1805,6 +1805,11 @@ static void spapr_machine_reset(void)
>>>  first_ppc_cpu->env.gpr[5] = 0;
>>>  
>>>  spapr->cas_reboot = false;
>>> +
>>> +spapr->guest_machine_check_addr = -1;
>>> +
>>> +/* Signal all vCPUs waiting on this condition */
>>> +qemu_cond_broadcast(>mc_delivery_cond);
>>>  }
>>>  
>>>  static void spapr_create_nvram(SpaprMachineState *spapr)
>>> @@ -2095,6 +2100,16 @@ static const VMStateDescription vmstate_spapr_dtb = {
>>>  },
>>>  };
>>>  
>>> +static const VMStateDescription vmstate_spapr_machine_check = {
>>> +.name = "spapr_machine_check",
>>> +.version_id = 1,
>>> +.minimum_version_id = 1,
>>> +.fields = (VMStateField[]) {
>>> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
> 
> Also this should use VMSTATE_UINTTL()

sure..

Regards,
Aravinda

> 
>>> +VMSTATE_END_OF_LIST()
>>> +},  
>>
>> This VMState descriptor is missing a .needed field because we only want
>> to migrate the subsection if the guest has called NMI register, ie.
>> spapr->guest_machine_check_addr != (target_ulong) -1.
>>
>>> +};
>>> +
>>>  static const VMStateDescription vmstate_spapr = 
>>> {765cf442a8afe8e5c8c6896b5072066df5129077
>>>  .name = "spapr",
>>>  .version_id = 3,
>>> @@ -2127,6 +2142,7 @@ static const VMStateDescription vmstate_spapr = {
>>>  _spapr_dtb,
>>>  _spapr_cap_large_decr,
>>>  _spapr_cap_ccf_assist,
>>> +_spapr_machine_check,
>>>  NULL
>>>  }
>>>  };
>>> @@ -3068,6 +3084,8 @@ static void spapr_machine_init(MachineState *machine)
>>>  
>>>  kvmppc_spapr_enable_inkernel_multitce();
>>>  }
>>> +
>>> +qemu_cond_init(>mc_delivery_cond);
>>>  }
>>>  
>>>  static int spapr_kvm_type(MachineState *machine, const char *vm_type)
>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>> index ee24212..c2f3991 100644
>>> --- a/hw/ppc/spapr_rtas.c
>>> +++ b/hw/ppc/spapr_rtas.c
>>> @@ -348,6 +348,39 @@ static void rtas_get_power_level(PowerPCCPU *cpu, 
>>> SpaprMachineState *spapr,
>>>  rtas_st(rets, 1, 100);
>>>  }
>>>  
>>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>> +  SpaprMachineState *spapr,
>>> +  uint32_t token, uint32_t nargs,
>>> +  target_ulong args,
>>> +  uint32_t nret, target_ulong rets)
>>> +{
>>> +uint64_t rtas_addr = spapr_get_rtas_addr();
>>> +
>>> +if (!rtas_addr) {
>>> +rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>> +return;
>>> +}
>>> +
>>> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
>>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>> +}
>>> +
>>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>>> +   SpaprMachineState *spapr,
>>> +   uint32_t token, uint32_t nargs,
>>> +   target_ulong args,
>>> +   uint32_t nret, target_ulong rets)
>>> +{
>>> +if (!spapr->guest_machine_check_addr) {  
>>
>> Hmm... the default value is -1. It looks like the check should rather be:
>>
>> if (spapr->guest_machine_check_addr == (target_ulong) -1) {
>>
>>
>>> +/* NMI register not called */
>>> +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>> +} else {
>>> +qemu_cond_signal(>mc_delivery_cond);
>>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>> +}
>>> +}
>>> 

Re: [Qemu-devel] [PATCH v8 2/6] virtio-pmem: Add virtio pmem driver

2019-05-12 Thread Pankaj Gupta


> > Guest reads the persistent memory range information from
> > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > creates a nd_region object with the persistent memory
> > range information so that existing 'nvdimm/pmem' driver
> > can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem
> > driver to register persistent memory compatible for DAX
> > capable filesystems.
> > 
> > This also provides function to perform guest flush over
> > VIRTIO from 'pmem' driver when userspace performs flush
> > on DAX memory range.
> > 
> > Signed-off-by: Pankaj Gupta 
> > Reviewed-by: Yuval Shaia 
> 
> Acked-by: Michael S. Tsirkin 

Thank you, Michael.

Best regards,
Pankaj

> 
> > ---
> >  drivers/nvdimm/Makefile  |   1 +
> >  drivers/nvdimm/nd_virtio.c   | 129 +++
> >  drivers/nvdimm/virtio_pmem.c | 117 
> >  drivers/virtio/Kconfig   |  10 +++
> >  include/linux/virtio_pmem.h  |  60 ++
> >  include/uapi/linux/virtio_ids.h  |   1 +
> >  include/uapi/linux/virtio_pmem.h |  10 +++
> >  7 files changed, 328 insertions(+)
> >  create mode 100644 drivers/nvdimm/nd_virtio.c
> >  create mode 100644 drivers/nvdimm/virtio_pmem.c
> >  create mode 100644 include/linux/virtio_pmem.h
> >  create mode 100644 include/uapi/linux/virtio_pmem.h
> > 
> > diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> > index 6f2a088afad6..cefe233e0b52 100644
> > --- a/drivers/nvdimm/Makefile
> > +++ b/drivers/nvdimm/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_ND_BTT) += nd_btt.o
> >  obj-$(CONFIG_ND_BLK) += nd_blk.o
> >  obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
> >  obj-$(CONFIG_OF_PMEM) += of_pmem.o
> > +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o nd_virtio.o
> >  
> >  nd_pmem-y := pmem.o
> >  
> > diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> > new file mode 100644
> > index ..ed7ddcc5a62c
> > --- /dev/null
> > +++ b/drivers/nvdimm/nd_virtio.c
> > @@ -0,0 +1,129 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + */
> > +#include 
> > +#include "nd.h"
> > +
> > + /* The interrupt handler */
> > +void host_ack(struct virtqueue *vq)
> > +{
> > +   unsigned int len;
> > +   unsigned long flags;
> > +   struct virtio_pmem_request *req, *req_buf;
> > +   struct virtio_pmem *vpmem = vq->vdev->priv;
> > +
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +   while ((req = virtqueue_get_buf(vq, )) != NULL) {
> > +   req->done = true;
> > +   wake_up(>host_acked);
> > +
> > +   if (!list_empty(>req_list)) {
> > +   req_buf = list_first_entry(>req_list,
> > +   struct virtio_pmem_request, list);
> > +   req_buf->wq_buf_avail = true;
> > +   wake_up(_buf->wq_buf);
> > +   list_del(_buf->list);
> > +   }
> > +   }
> > +   spin_unlock_irqrestore(>pmem_lock, flags);
> > +}
> > +EXPORT_SYMBOL_GPL(host_ack);
> > +
> > + /* The request submission function */
> > +int virtio_pmem_flush(struct nd_region *nd_region)
> > +{
> > +   int err, err1;
> > +   unsigned long flags;
> > +   struct scatterlist *sgs[2], sg, ret;
> > +   struct virtio_device *vdev = nd_region->provider_data;
> > +   struct virtio_pmem *vpmem = vdev->priv;
> > +   struct virtio_pmem_request *req;
> > +
> > +   might_sleep();
> > +   req = kmalloc(sizeof(*req), GFP_KERNEL);
> > +   if (!req)
> > +   return -ENOMEM;
> > +
> > +   req->done = false;
> > +   strcpy(req->name, "FLUSH");
> > +   init_waitqueue_head(>host_acked);
> > +   init_waitqueue_head(>wq_buf);
> > +   INIT_LIST_HEAD(>list);
> > +   sg_init_one(, req->name, strlen(req->name));
> > +   sgs[0] = 
> > +   sg_init_one(, >ret, sizeof(req->ret));
> > +   sgs[1] = 
> > +
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +/*
> > + * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
> > + * queue does not have free descriptor. We add the request
> > + * to req_list and wait for host_ack to wake us up when free
> > + * slots are available.
> > + */
> > +   while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req,
> > +   GFP_ATOMIC)) == -ENOSPC) {
> > +
> > +   dev_err(>dev, "failed to send command to virtio pmem"\
> > +   "device, no free slots in the virtqueue\n");
> > +   req->wq_buf_avail = false;
> > +   list_add_tail(>list, >req_list);
> > +   spin_unlock_irqrestore(>pmem_lock, flags);
> > +
> > +   /* When host has read buffer, this completes via host_ack */
> > +   wait_event(req->wq_buf, req->wq_buf_avail);
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +   }
> > +  

Re: [Qemu-devel] [Qemu-ppc] [PATCH v8 1/6] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

2019-05-12 Thread Aravinda Prasad



On Friday 10 May 2019 02:36 PM, Greg Kurz wrote:
> On Mon, 22 Apr 2019 12:32:58 +0530
> Aravinda Prasad  wrote:
> 
>> This patch adds support in QEMU to handle "ibm,nmi-register"
>> and "ibm,nmi-interlock" RTAS calls.
>>
>> The machine check notification address is saved when the
>> OS issues "ibm,nmi-register" RTAS call.
>>
>> This patch also handles the case when multiple processors
>> experience machine check at or about the same time by
>> handling "ibm,nmi-interlock" call. In such cases, as per
>> PAPR, subsequent processors serialize waiting for the first
>> processor to issue the "ibm,nmi-interlock" call. The second
>> processor that also received a machine check error waits
>> till the first processor is done reading the error log.
>> The first processor issues "ibm,nmi-interlock" call
>> when the error log is consumed. This patch implements the
>> releasing part of the error-log while subsequent patch
>> (which builds error log) handles the locking part.
>>
>> Signed-off-by: Aravinda Prasad 
>> ---
>>  hw/ppc/spapr.c |   18 ++
>>  hw/ppc/spapr_rtas.c|   61 
>> 
>>  include/hw/ppc/spapr.h |9 ++-
>>  3 files changed, 87 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index c56939a..6642cb5 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1805,6 +1805,11 @@ static void spapr_machine_reset(void)
>>  first_ppc_cpu->env.gpr[5] = 0;
>>  
>>  spapr->cas_reboot = false;
>> +
>> +spapr->guest_machine_check_addr = -1;
>> +
>> +/* Signal all vCPUs waiting on this condition */
>> +qemu_cond_broadcast(>mc_delivery_cond);
>>  }
>>  
>>  static void spapr_create_nvram(SpaprMachineState *spapr)
>> @@ -2095,6 +2100,16 @@ static const VMStateDescription vmstate_spapr_dtb = {
>>  },
>>  };
>>  
>> +static const VMStateDescription vmstate_spapr_machine_check = {
>> +.name = "spapr_machine_check",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
>> +VMSTATE_END_OF_LIST()
>> +},
> 
> This VMState descriptor is missing a .needed field because we only want
> to migrate the subsection if the guest has called NMI register, ie.
> spapr->guest_machine_check_addr != (target_ulong) -1.

Ok.. let me check.

> 
>> +};
>> +
>>  static const VMStateDescription vmstate_spapr = {
>>  .name = "spapr",
>>  .version_id = 3,
>> @@ -2127,6 +2142,7 @@ static const VMStateDescription vmstate_spapr = {
>>  _spapr_dtb,
>>  _spapr_cap_large_decr,
>>  _spapr_cap_ccf_assist,
>> +_spapr_machine_check,
>>  NULL
>>  }
>>  };
>> @@ -3068,6 +3084,8 @@ static void spapr_machine_init(MachineState *machine)
>>  
>>  kvmppc_spapr_enable_inkernel_multitce();
>>  }
>> +
>> +qemu_cond_init(>mc_delivery_cond);
>>  }
>>  
>>  static int spapr_kvm_type(MachineState *machine, const char *vm_type)
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index ee24212..c2f3991 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -348,6 +348,39 @@ static void rtas_get_power_level(PowerPCCPU *cpu, 
>> SpaprMachineState *spapr,
>>  rtas_st(rets, 1, 100);
>>  }
>>  
>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> +  SpaprMachineState *spapr,
>> +  uint32_t token, uint32_t nargs,
>> +  target_ulong args,
>> +  uint32_t nret, target_ulong rets)
>> +{
>> +uint64_t rtas_addr = spapr_get_rtas_addr();
>> +
>> +if (!rtas_addr) {
>> +rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> +return;
>> +}
>> +
>> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +
>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> +   SpaprMachineState *spapr,
>> +   uint32_t token, uint32_t nargs,
>> +   target_ulong args,
>> +   uint32_t nret, target_ulong rets)
>> +{
>> +if (!spapr->guest_machine_check_addr) {
> 
> Hmm... the default value is -1. It looks like the check should rather be:
> 
> if (spapr->guest_machine_check_addr == (target_ulong) -1) {

ok..

> 
> 
>> +/* NMI register not called */
>> +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> +} else {
>> +qemu_cond_signal(>mc_delivery_cond);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +}
>> +
>> +
>>  static struct rtas_call {
>>  const char *name;
>>  spapr_rtas_fn fn;
>> @@ -466,6 +499,30 @@ void spapr_load_rtas(SpaprMachineState *spapr, void 
>> *fdt, hwaddr addr)
>>  }
>>  }
>>  
>> +uint64_t spapr_get_rtas_addr(void)
> 
> Shouldn't this be hwaddr 

Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration

2019-05-12 Thread Catherine Ho
Sorry for the noise, is there any more comment for this patch?
Without this patch, ignore shared capabilities can not be used on arm64

B.R.
Catherine

On Tue, 16 Apr 2019 at 10:51, Peter Xu  wrote:

> On Mon, Apr 08, 2019 at 04:42:13AM -0400, Catherine Ho wrote:
> > Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> > addes ignore-shared capability to bypass the shared ramblock (e,g,
> > membackend + numa node). It does good to live migration.
> >
> > As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> > until VM starts, but it does on aarch64 qemu:
> > Backtrace:
> > 1  0x55f4a296dd84 in address_space_write_rom_internal () at
> > exec.c:3458
> > 2  0x55f4a296de3a in address_space_write_rom () at exec.c:3479
> > 3  0x55f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> > 4  0x55f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> > 5  0x55f4a2c90a28 in qemu_system_reset () at vl.c:1675
> > 6  0x55f4a2c9851d in main () at vl.c:4552
> >
> > Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> > druing rom_reset. In ignore-shared incoming case, this rom filling
> > is not required since all the data has been stored in memory backend
> > file.
> >
> > Further more, as suggested by Peter Xu, if we do rom_reset() now with
> > these ROMs then the RAM data should be re-filled again too with the
> > migration stream coming in.
> >
> > Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> > capability")
> > Suggested-by: Yury Kotov 
> > Suggested-by: Peter Xu 
> > Signed-off-by: Catherine Ho 
>
> Acked-by: Peter Xu 
>
> --
> Peter Xu
>


Re: [Qemu-devel] [PATCH v6 24/25] target/ppc: Use gen_io_start/end around DARN

2019-05-12 Thread David Gibson
On Fri, May 10, 2019 at 10:30:48AM -0700, Richard Henderson wrote:
> Generating a random number counts as I/O, as it cannot be
> replayed and produce the same results.
> 
> Cc: David Gibson 
> Suggested-by: Peter Maydell 
> Signed-off-by: Richard Henderson 

Acked-by: David Gibson 

> ---
>  target/ppc/translate.c | 21 +++--
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 8d08625c33..76628df6dd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1847,13 +1847,22 @@ static void gen_darn(DisasContext *ctx)
>  {
>  int l = L(ctx->opcode);
>  
> -if (l == 0) {
> -gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
> -} else if (l <= 2) {
> -/* Return 64-bit random for both CRN and RRN */
> -gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
> -} else {
> +if (l > 2) {
>  tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
> +} else {
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_start();
> +}
> +if (l == 0) {
> +gen_helper_darn32(cpu_gpr[rD(ctx->opcode)]);
> +} else {
> +/* Return 64-bit random for both CRN and RRN */
> +gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
> +}
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_end();
> +gen_stop_exception(ctx);
> +}
>  }
>  }
>  #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 v2 1/2] vfio/mdev: add version attribute for mdev device

2019-05-12 Thread Yan Zhao
On Fri, May 10, 2019 at 05:48:38PM +0800, Cornelia Huck wrote:
> On Fri, 10 May 2019 10:36:09 +0100
> "Dr. David Alan Gilbert"  wrote:
> 
> > * Cornelia Huck (coh...@redhat.com) wrote:
> > > On Thu, 9 May 2019 17:48:26 +0100
> > > "Dr. David Alan Gilbert"  wrote:
> > >   
> > > > * Cornelia Huck (coh...@redhat.com) wrote:  
> > > > > On Thu, 9 May 2019 16:48:57 +0100
> > > > > "Dr. David Alan Gilbert"  wrote:
> > > > > 
> > > > > > * Cornelia Huck (coh...@redhat.com) wrote:
> > > > > > > On Tue, 7 May 2019 15:18:26 -0600
> > > > > > > Alex Williamson  wrote:
> > > > > > >   
> > > > > > > > On Sun,  5 May 2019 21:49:04 -0400
> > > > > > > > Yan Zhao  wrote:  
> > > > > > >   
> > > > > > > > > +  Errno:
> > > > > > > > > +  If vendor driver wants to claim a mdev device incompatible 
> > > > > > > > > to all other mdev
> > > > > > > > > +  devices, it should not register version attribute for this 
> > > > > > > > > mdev device. But if
> > > > > > > > > +  a vendor driver has already registered version attribute 
> > > > > > > > > and it wants to claim
> > > > > > > > > +  a mdev device incompatible to all other mdev devices, it 
> > > > > > > > > needs to return
> > > > > > > > > +  -ENODEV on access to this mdev device's version attribute.
> > > > > > > > > +  If a mdev device is only incompatible to certain mdev 
> > > > > > > > > devices, write of
> > > > > > > > > +  incompatible mdev devices's version strings to its version 
> > > > > > > > > attribute should
> > > > > > > > > +  return -EINVAL;
> > > > > > > > 
> > > > > > > > I think it's best not to define the specific errno returned for 
> > > > > > > > a
> > > > > > > > specific situation, let the vendor driver decide, userspace 
> > > > > > > > simply
> > > > > > > > needs to know that an errno on read indicates the device does 
> > > > > > > > not
> > > > > > > > support migration version comparison and that an errno on write
> > > > > > > > indicates the devices are incompatible or the target doesn't 
> > > > > > > > support
> > > > > > > > migration versions.  
> > > > > > > 
> > > > > > > I think I have to disagree here: It's probably valuable to have an
> > > > > > > agreed error for 'cannot migrate at all' vs 'cannot migrate 
> > > > > > > between
> > > > > > > those two particular devices'. Userspace might want to do 
> > > > > > > different
> > > > > > > things (e.g. trying with different device pairs).  
> > > > > > 
> > > > > > Trying to stuff these things down an errno seems a bad idea; we 
> > > > > > can't
> > > > > > get much information that way.
> > > > > 
> > > > > So, what would be a reasonable approach? Userspace should first read
> > > > > the version attributes on both devices (to find out whether migration
> > > > > is supported at all), and only then figure out via writing whether 
> > > > > they
> > > > > are compatible?
> > > > > 
> > > > > (Or just go ahead and try, if it does not care about the reason.)
> > > > 
> > > > Well, I'm OK with something like writing to test whether it's
> > > > compatible, it's just we need a better way of saying 'no'.
> > > > I'm not sure if that involves reading back from somewhere after
> > > > the write or what.  
> > > 
> > > Hm, so I basically see two ways of doing that:
> > > - standardize on some error codes... problem: error codes can be hard
> > >   to fit to reasons
> > > - make the error available in some attribute that can be read
> > > 
> > > I'm not sure how we can serialize the readback with the last write,
> > > though (this looks inherently racy).
> > > 
> > > How important is detailed error reporting here?  
> > 
> > I think we need something, otherwise we're just going to get vague
> > user reports of 'but my VM doesn't migrate'; I'd like the error to be
> > good enough to point most users to something they can understand
> > (e.g. wrong card family/too old a driver etc).
> 
> Ok, that sounds like a reasonable point. Not that I have a better idea
> how to achieve that, though... we could also log a more verbose error
> message to the kernel log, but that's not necessarily where a user will
> look first.
> 
> Ideally, we'd want to have the user space program setting up things
> querying the general compatibility for migration (so that it becomes
> their problem on how to alert the user to problems :), but I'm not sure
> how to eliminate the race between asking the vendor driver for
> compatibility and getting the result of that operation.
> 
> Unless we introduce an interface that can retrieve _all_ results
> together with the written value? Or is that not going to be much of a
> problem in practice?
what about defining a migration_errors attribute, storing recent 10 error
records with format like:
input string: error
as identical input strings always have the same error string, the 10 error
records may meet 10+ reason querying operations. And in practice, I think there
wouldn't be 10 simultaneous migration requests?

or 

Re: [Qemu-devel] [PATCH 1/5] vfio: vfio_iommu_type1: linux header place holder

2019-05-12 Thread Michael S. Tsirkin
On Fri, May 10, 2019 at 04:38:49PM +0200, Pierre Morel wrote:
> This should be copied from Linux kernel UAPI includes.
> 
> Signed-off-by: Pierre Morel 

pls add a note which linux version did you sync with.

> ---
>  linux-headers/linux/vfio.h | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> index 12a7b1d..eaecaef 100644
> --- a/linux-headers/linux/vfio.h
> +++ b/linux-headers/linux/vfio.h
> @@ -9,8 +9,8 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> -#ifndef VFIO_H
> -#define VFIO_H
> +#ifndef _UAPIVFIO_H
> +#define _UAPIVFIO_H
>  
>  #include 
>  #include 
> @@ -711,6 +711,16 @@ struct vfio_iommu_type1_info {
>   __u32   flags;
>  #define VFIO_IOMMU_INFO_PGSIZES (1 << 0) /* supported page sizes info */
>   __u64   iova_pgsizes;   /* Bitmap of supported page sizes */
> +#define VFIO_IOMMU_INFO_CAPABILITIES (1 << 1)  /* support capabilities info 
> */
> + __u64   cap_offset; /* Offset within info struct of first cap */
> +};
> +
> +#define VFIO_IOMMU_INFO_CAP_QFN  1
> +#define VFIO_IOMMU_INFO_CAP_QGRP 2
> +
> +struct vfio_iommu_type1_info_block {
> + struct vfio_info_cap_header header;
> + __u32 data[];
>  };
>  
>  #define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
> @@ -910,4 +920,4 @@ struct vfio_iommu_spapr_tce_remove {
>  
>  /* * */
>  
> -#endif /* VFIO_H */
> +#endif /* _UAPIVFIO_H */
> -- 
> 2.7.4



Re: [Qemu-devel] [PATCH v3 0/3] rng-builtin: add an RNG backend that uses qemu_guest_getrandom()

2019-05-12 Thread Michael S. Tsirkin
On Fri, May 10, 2019 at 03:42:00PM +0200, Laurent Vivier wrote:
> Add a new RNG backend using QEMU builtin getrandom function.
> 
> This patch applies on top of
> "[PATCH v5 00/24] Add qemu_getrandom and ARMv8.5-RNG etc"
> Based-on: 20190510012458.22706-1-richard.hender...@linaro.org
> 
> v3: Include Kashyap's patch in the series
> Add a patch to change virtio-rng default backend to rng-builtin
> 
> v2: Update qemu-options.hx
> describe the new backend and specify virtio-rng uses the
> rng-random by default
> 
> Kashyap Chamarthy (1):
>   VirtIO-RNG: Update default entropy source to `/dev/urandom`
> 
> Laurent Vivier (2):
>   rng-builtin: add an RNG backend that uses qemu_guest_getrandom()
>   virtio-rng: change default backend to rng-builtin


OK pls address Marku's comment on commit msg and I will merge.

>  backends/Makefile.objs |  2 +-
>  backends/rng-builtin.c | 54 ++
>  backends/rng-random.c  |  2 +-
>  hw/virtio/virtio-rng.c |  2 +-
>  include/hw/virtio/virtio-rng.h |  4 +--
>  include/sysemu/rng-builtin.h   | 17 +++
>  qemu-options.hx|  9 +-
>  7 files changed, 84 insertions(+), 6 deletions(-)
>  create mode 100644 backends/rng-builtin.c
>  create mode 100644 include/sysemu/rng-builtin.h
> 
> -- 
> 2.20.1



Re: [Qemu-devel] [PATCH v4 01/15] tests: acpi: rename acpi_parse_rsdp_table() into acpi_fetch_rsdp_table()

2019-05-12 Thread Michael S. Tsirkin
On Thu, May 02, 2019 at 04:51:49PM +0200, Igor Mammedov wrote:
> so name would reflect what the function does
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Wei Yang 
> ---
> v4:
>  * make it as the first patch in series
> ---


FYI this trips up git am.
Don't do two --- please: just one is enough,
second is not needed.


>  tests/acpi-utils.h   | 2 +-
>  tests/acpi-utils.c   | 2 +-
>  tests/bios-tables-test.c | 2 +-
>  tests/vmgenid-test.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> index ef388bb..4cd5553 100644
> --- a/tests/acpi-utils.h
> +++ b/tests/acpi-utils.h
> @@ -47,7 +47,7 @@ typedef struct {
>  uint8_t acpi_calc_checksum(const uint8_t *data, int len);
>  uint32_t acpi_find_rsdp_address(QTestState *qts);
>  uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table);
> -void acpi_parse_rsdp_table(QTestState *qts, uint32_t addr, uint8_t 
> *rsdp_table);
> +void acpi_fetch_rsdp_table(QTestState *qts, uint32_t addr, uint8_t 
> *rsdp_table);
>  void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len,
>const uint8_t *addr_ptr, const char *sig,
>bool verify_checksum);
> diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c
> index cc33b46..633d8f5 100644
> --- a/tests/acpi-utils.c
> +++ b/tests/acpi-utils.c
> @@ -63,7 +63,7 @@ uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table)
>  return le64_to_cpu(xsdt_physical_address);
>  }
>  
> -void acpi_parse_rsdp_table(QTestState *qts, uint32_t addr, uint8_t 
> *rsdp_table)
> +void acpi_fetch_rsdp_table(QTestState *qts, uint32_t addr, uint8_t 
> *rsdp_table)
>  {
>  uint8_t revision;
>  
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index a506dcb..6a678bf 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -89,7 +89,7 @@ static void test_acpi_rsdp_table(test_data *data)
>  uint8_t *rsdp_table = data->rsdp_table, revision;
>  uint32_t addr = data->rsdp_addr;
>  
> -acpi_parse_rsdp_table(data->qts, addr, rsdp_table);
> +acpi_fetch_rsdp_table(data->qts, addr, rsdp_table);
>  revision = rsdp_table[15 /* Revision offset */];
>  
>  switch (revision) {
> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> index ae38ee5..f400184 100644
> --- a/tests/vmgenid-test.c
> +++ b/tests/vmgenid-test.c
> @@ -40,7 +40,7 @@ static uint32_t acpi_find_vgia(QTestState *qts)
>  g_assert_cmphex(rsdp_offset, <, RSDP_ADDR_INVALID);
>  
>  
> -acpi_parse_rsdp_table(qts, rsdp_offset, rsdp_table);
> +acpi_fetch_rsdp_table(qts, rsdp_offset, rsdp_table);
>  acpi_fetch_table(qts, , _len, _table[16 /* RsdtAddress 
> */],
>   "RSDT", true);
>  
> -- 
> 2.7.4

On Thu, May 02, 2019 at 04:51:50PM +0200, Igor Mammedov wrote:
> Currently acpi_fetch_table() assumes 32 bit size of table pointer
> in ACPI tables. However X_foo variants are 64 bit, prepare
> acpi_fetch_table() to handle both by adding an argument
> for addr_ptr pointed entry size. Follow up commits will use that
> to read XSDT and X_foo entries in ACPI tables.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  tests/acpi-utils.h   |  2 +-
>  tests/acpi-utils.c   | 10 ++
>  tests/bios-tables-test.c |  8 
>  tests/vmgenid-test.c |  4 ++--
>  4 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> index 4cd5553..92285b7 100644
> --- a/tests/acpi-utils.h
> +++ b/tests/acpi-utils.h
> @@ -49,7 +49,7 @@ uint32_t acpi_find_rsdp_address(QTestState *qts);
>  uint64_t acpi_get_xsdt_address(uint8_t *rsdp_table);
>  void acpi_fetch_rsdp_table(QTestState *qts, uint32_t addr, uint8_t 
> *rsdp_table);
>  void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len,
> -  const uint8_t *addr_ptr, const char *sig,
> +  const uint8_t *addr_ptr, int addr_size, const char 
> *sig,
>bool verify_checksum);
>  
>  #endif  /* TEST_ACPI_UTILS_H */
> diff --git a/tests/acpi-utils.c b/tests/acpi-utils.c
> index 633d8f5..644c87b 100644
> --- a/tests/acpi-utils.c
> +++ b/tests/acpi-utils.c
> @@ -91,13 +91,15 @@ void acpi_fetch_rsdp_table(QTestState *qts, uint32_t 
> addr, uint8_t *rsdp_table)
>   *  actual one.
>   */
>  void acpi_fetch_table(QTestState *qts, uint8_t **aml, uint32_t *aml_len,
> -  const uint8_t *addr_ptr, const char *sig,
> +  const uint8_t *addr_ptr, int addr_size, const char 
> *sig,
>bool verify_checksum)
>  {
> -uint32_t addr, len;
> +uint32_t len;
> +uint64_t addr = 0;
>  
> -memcpy(, addr_ptr , sizeof(addr));
> -addr = le32_to_cpu(addr);
> +g_assert(addr_size == 4 || addr_size == 8);
> +memcpy(, addr_ptr , addr_size);
> +addr = le64_to_cpu(addr);
>  qtest_memread(qts, addr + 4, , 4); /* Length 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/3] pcie: Simplify pci_adjust_config_limit()

2019-05-12 Thread Michael S. Tsirkin
On Tue, May 07, 2019 at 02:48:38PM +1000, David Gibson wrote:
> On Fri, Apr 26, 2019 at 04:40:17PM +1000, Alexey Kardashevskiy wrote:
> > 
> > 
> > On 24/04/2019 14:19, David Gibson wrote:
> > > Since c2077e2c "pci: Adjust PCI config limit based on bus topology",
> > > pci_adjust_config_limit() has been used in the config space read and write
> > > paths to only permit access to extended config space on buses which permit
> > > it.  Specifically it prevents access on devices below a vanilla-PCI bus 
> > > via
> > > some combination of bridges, even if both the host bridge and the device
> > > itself are PCI-E.
> > > 
> > > It accomplishes this with a somewhat complex call up the chain of bridges
> > > to see if any of them prohibit extended config space access.  This is
> > > overly complex, since we can always know if the bus will support such
> > > access at the point it is constructed.
> > > 
> > > This patch simplifies the test by using a flag in the PCIBus instance
> > > indicating whether extended configuration space is accessible.  It is
> > > false for vanilla PCI buses.  For PCI-E buses, it is true for root
> > > buses and equal to the parent bus's's capability otherwise.
> > > 
> > > For the special case of sPAPR's paravirtualized PCI root bus, which
> > > acts mostly like vanilla PCI, but does allow extended config space
> > > access, we override the default value of the flag from the host bridge
> > > code.
> > > 
> > > This should cause no behavioural change.
> > > 
> > > Signed-off-by: David Gibson cd
> > > ---
> > >  hw/pci/pci.c | 41 ++--
> > >  hw/pci/pci_host.c| 13 +++--
> > >  hw/ppc/spapr_pci.c   | 34 ++---
> > >  include/hw/pci/pci.h |  1 -
> > >  include/hw/pci/pci_bus.h |  9 -
> > >  5 files changed, 44 insertions(+), 54 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index ea5941fb22..59ee034331 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -120,6 +120,27 @@ static void pci_bus_realize(BusState *qbus, Error 
> > > **errp)
> > >  vmstate_register(NULL, -1, _pcibus, bus);
> > >  }
> > >  
> > > +static void pcie_bus_realize(BusState *qbus, Error **errp)
> > > +{
> > > +PCIBus *bus = PCI_BUS(qbus);
> > > +
> > > +pci_bus_realize(qbus, errp);
> > > +
> > > +/*
> > > + * A PCI-E bus can support extended config space if it's the root
> > > + * bus, or if the bus/bridge above it does as well
> > > + */
> > > +if (pci_bus_is_root(bus)) {
> > > +bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
> > > +} else {
> > > +PCIBus *parent_bus = pci_get_bus(bus->parent_dev);
> > 
> > 
> > g_assert(bus->parent_dev) ?
> > 
> > Slightly confusingly bus->parent_dev is not the same as bus->qbus.parent
> > and can be NULL, I'd even look into ditching parent_dev and using
> > bus->qbus.parent instead (if possible).
> 
> Oh boy, the can of worms I reached into following up that simple
> comment.  Yes, they're subtly different, and yes it's confusing.  In
> practice parent_dev is NULL when on a root bus, that's not a PXB bus,
> and otherwise equal to qbus.parent.
> 
> After a *lot* of thinking about this, I think parent_dev is actually
> correct here - we're explicitly looking at the parent as a P2P bridge,
> not anything else.
> 
> But, I'll try to do some later cleanups making the parent_dev /
> qbus.parent confusion a bit better.
> > > +static inline bool pci_bus_allows_extended_config_space(PCIBus *bus)
> > > +{
> > > +return !!(bus->flags & PCI_BUS_EXTENDED_CONFIG_SPACE);
> > > +}
> > > +
> > > +
> > 
> > An extra empty line.
> > 
> > Anyway, these are minor comments, so
> > 
> > Reviewed-by: Alexey Kardashevskiy 
> > 
> > 
> > 
> > 
> > >  #endif /* QEMU_PCI_BUS_H */
> > > 
> > 

Pls address comments and repost ok?

> -- 
> 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





Re: [Qemu-devel] [PATCH v8 2/6] virtio-pmem: Add virtio pmem driver

2019-05-12 Thread Michael S. Tsirkin
On Fri, May 10, 2019 at 09:21:58PM +0530, Pankaj Gupta wrote:
> This patch adds virtio-pmem driver for KVM guest.
> 
> Guest reads the persistent memory range information from
> Qemu over VIRTIO and registers it on nvdimm_bus. It also
> creates a nd_region object with the persistent memory
> range information so that existing 'nvdimm/pmem' driver
> can reserve this into system memory map. This way
> 'virtio-pmem' driver uses existing functionality of pmem
> driver to register persistent memory compatible for DAX
> capable filesystems.
> 
> This also provides function to perform guest flush over
> VIRTIO from 'pmem' driver when userspace performs flush
> on DAX memory range.
> 
> Signed-off-by: Pankaj Gupta 
> Reviewed-by: Yuval Shaia 

Acked-by: Michael S. Tsirkin 

> ---
>  drivers/nvdimm/Makefile  |   1 +
>  drivers/nvdimm/nd_virtio.c   | 129 +++
>  drivers/nvdimm/virtio_pmem.c | 117 
>  drivers/virtio/Kconfig   |  10 +++
>  include/linux/virtio_pmem.h  |  60 ++
>  include/uapi/linux/virtio_ids.h  |   1 +
>  include/uapi/linux/virtio_pmem.h |  10 +++
>  7 files changed, 328 insertions(+)
>  create mode 100644 drivers/nvdimm/nd_virtio.c
>  create mode 100644 drivers/nvdimm/virtio_pmem.c
>  create mode 100644 include/linux/virtio_pmem.h
>  create mode 100644 include/uapi/linux/virtio_pmem.h
> 
> diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
> index 6f2a088afad6..cefe233e0b52 100644
> --- a/drivers/nvdimm/Makefile
> +++ b/drivers/nvdimm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_ND_BTT) += nd_btt.o
>  obj-$(CONFIG_ND_BLK) += nd_blk.o
>  obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
>  obj-$(CONFIG_OF_PMEM) += of_pmem.o
> +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o nd_virtio.o
>  
>  nd_pmem-y := pmem.o
>  
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> new file mode 100644
> index ..ed7ddcc5a62c
> --- /dev/null
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * virtio_pmem.c: Virtio pmem Driver
> + *
> + * Discovers persistent memory range information
> + * from host and provides a virtio based flushing
> + * interface.
> + */
> +#include 
> +#include "nd.h"
> +
> + /* The interrupt handler */
> +void host_ack(struct virtqueue *vq)
> +{
> + unsigned int len;
> + unsigned long flags;
> + struct virtio_pmem_request *req, *req_buf;
> + struct virtio_pmem *vpmem = vq->vdev->priv;
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> + while ((req = virtqueue_get_buf(vq, )) != NULL) {
> + req->done = true;
> + wake_up(>host_acked);
> +
> + if (!list_empty(>req_list)) {
> + req_buf = list_first_entry(>req_list,
> + struct virtio_pmem_request, list);
> + req_buf->wq_buf_avail = true;
> + wake_up(_buf->wq_buf);
> + list_del(_buf->list);
> + }
> + }
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(host_ack);
> +
> + /* The request submission function */
> +int virtio_pmem_flush(struct nd_region *nd_region)
> +{
> + int err, err1;
> + unsigned long flags;
> + struct scatterlist *sgs[2], sg, ret;
> + struct virtio_device *vdev = nd_region->provider_data;
> + struct virtio_pmem *vpmem = vdev->priv;
> + struct virtio_pmem_request *req;
> +
> + might_sleep();
> + req = kmalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + req->done = false;
> + strcpy(req->name, "FLUSH");
> + init_waitqueue_head(>host_acked);
> + init_waitqueue_head(>wq_buf);
> + INIT_LIST_HEAD(>list);
> + sg_init_one(, req->name, strlen(req->name));
> + sgs[0] = 
> + sg_init_one(, >ret, sizeof(req->ret));
> + sgs[1] = 
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> +  /*
> +   * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
> +   * queue does not have free descriptor. We add the request
> +   * to req_list and wait for host_ack to wake us up when free
> +   * slots are available.
> +   */
> + while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req,
> + GFP_ATOMIC)) == -ENOSPC) {
> +
> + dev_err(>dev, "failed to send command to virtio pmem"\
> + "device, no free slots in the virtqueue\n");
> + req->wq_buf_avail = false;
> + list_add_tail(>list, >req_list);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /* When host has read buffer, this completes via host_ack */
> + wait_event(req->wq_buf, req->wq_buf_avail);
> + spin_lock_irqsave(>pmem_lock, flags);
> + }
> + err1 = virtqueue_kick(vpmem->req_vq);
> + spin_unlock_irqrestore(>pmem_lock, 

Re: [Qemu-devel] [PATCH v8 0/6] virtio pmem driver

2019-05-12 Thread Michael S. Tsirkin
On Fri, May 10, 2019 at 07:33:03PM -0400, Pankaj Gupta wrote:
> 
> > >
> > >  Hi Michael & Dan,
> > >
> > >  Please review/ack the patch series from LIBNVDIMM & VIRTIO side.
> > >  We have ack on ext4, xfs patches(4, 5 & 6) patch 2. Still need
> > >  your ack on nvdimm patches(1 & 3) & virtio patch 2.
> > 
> > I was planning to merge these via the nvdimm tree, not ack them. Did
> > you have another maintainer lined up to take these patches?
> 
> Sorry! for not being clear on this. I wanted to say same.
> 
> Proposed the patch series to be merged via nvdimm tree as kindly agreed
> by you. We only need an ack on virtio patch 2 from Micahel.
> 
> Thank you for all your help.
> 
> Best regards,
> Pankaj Gupta

Fine by me.

> > 



[Qemu-devel] [PATCH] block: Make block error codes consistent

2019-05-12 Thread Wensheng Tang
We should keep the error handling consistent. ENOMEDIUM is more meaningful than 
ENOENT a when driver cannot be loaded.
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 5c2c6aa..6685be7 100644
--- a/block.c
+++ b/block.c
@@ -770,7 +770,7 @@ static int find_image_format(BlockBackend *file, const char 
*filename,
 if (!drv) {
 error_setg(errp, "Could not determine image format: No compatible "
"driver found");
-ret = -ENOENT;
+ret = -ENOMEDIUM;
 }
 *pdrv = drv;
 return ret;
@@ -1619,7 +1619,7 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 drv = bdrv_find_format(drvname);
 if (!drv) {
 error_setg(errp, "Unknown driver '%s'", drvname);
-return -ENOENT;
+return -ENOMEDIUM;
 }
 /* If the user has explicitly specified the driver, this choice should
  * override the BDRV_O_PROTOCOL flag */
@@ -1655,7 +1655,7 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 if (filename) {
 drv = bdrv_find_protocol(filename, parse_filename, errp);
 if (!drv) {
-return -EINVAL;
+return -ENOMEDIUM;
 }
 
 drvname = drv->format_name;
-- 
2.20.1 (Apple Git-117)




[Qemu-devel] [PATCH] block: Make block error codes consistent

2019-05-12 Thread Wensheng Tang
(The last email does not include signed-off-by line. Please ignoreit)

We should keep the error handling consistent. ENOMEDIUM is more meaningful than 
ENOENT a when driver cannot be loaded.

Signed-off-by: Wensheng Tang 
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 5c2c6aa..6685be7 100644
--- a/block.c
+++ b/block.c
@@ -770,7 +770,7 @@ static int find_image_format(BlockBackend *file, const char 
*filename,
 if (!drv) {
 error_setg(errp, "Could not determine image format: No compatible "
"driver found");
-ret = -ENOENT;
+ret = -ENOMEDIUM;
 }
 *pdrv = drv;
 return ret;
@@ -1619,7 +1619,7 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 drv = bdrv_find_format(drvname);
 if (!drv) {
 error_setg(errp, "Unknown driver '%s'", drvname);
-return -ENOENT;
+return -ENOMEDIUM;
 }
 /* If the user has explicitly specified the driver, this choice should
  * override the BDRV_O_PROTOCOL flag */
@@ -1655,7 +1655,7 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 if (filename) {
 drv = bdrv_find_protocol(filename, parse_filename, errp);
 if (!drv) {
-return -EINVAL;
+return -ENOMEDIUM;
 }
 
 drvname = drv->format_name;
-- 
2.20.1 (Apple Git-117)




Re: [Qemu-devel] [PATCH] configure: Change capstone's default state to disabled

2019-05-12 Thread Thomas Huth
On 11/05/2019 20.28, Programmingkid wrote:
> 
>> On May 11, 2019, at 2:05 PM, Thomas Huth  wrote:
>>
>> On 11/05/2019 19.21, Programmingkid wrote:
>>>
 On Apr 20, 2019, at 6:40 AM, Thomas Huth  wrote:

 On 19/04/2019 15.44, G 3 wrote:
[...]
> Thank you for replying. Capstone comes with QEMU? Every time I try to
> compile QEMU I see an error relating to Capstone not being on my system.
> Why do you feel that disabling Capstone by default is not a good idea?
>
> Here is the error message I see when compiling QEMU:
>
> CHK version_gen.h
> make[1]: *** No rule to make target
> `/Users/John/qemu-git/capstone/libcapstone.a'.  Stop.
> make: *** [subdir-capstone] Error 2

 I assume you're using a git checkout here, right? For git checkouts, the
 Makefile should take care of calling the scripts/git-submodule.sh script
 which should initialize the submodule in the capstone directory.

 What's the content of your .git-submodule-status file? What does
 "configure" say about capstone support on your system?

 Thomas
>>>
>>> Yes I use a git checkout.
>>>
>>> This is the contents of my .git-submodule-status file:
>>> #!/bin/sh
>> [...]
>>
>> That were the contents of scripts/git-submodule.sh. I meant the hidden
>> file .git-submodule-status in the main directory.
> 
> This is it:
>  88f18909db731a627456f26d779445f84e449536 dtc (v1.4.7)
>  f0da6726207b740f6101028b2992f918477a4b08 slirp (v4.0.0-rc0-25-gf0da672)
>  b64af41c3276f97f0e181920400ee056b9c88037 tests/fp/berkeley-softfloat-3 
> (heads/master)
>  5a59dcec19327396a011a17fd924aed4fec416b3 tests/fp/berkeley-testfloat-3 
> (remotes/origin/HEAD)
>  6b3d716e2b6472eb7189d3220552280ef3d832ce ui/keycodemapdb 
> (heads/master-4-g6b3d716)

There should be an entry for capstone in here, too. :-/

>>> I did a 'make clean' followed by a 'make distclean'. Then tried building 
>>> again using this command line:
>>>
>>> ./configure --target-list=ppc-softmmu,i386-softmmu,x86_64-softmmu
>>> make -j 4
>>
>> That should normally populate the capstone directory. What happens if
>> you run "make git-submodule-update" directly?
> 
> Here is the result:
> $ make git-submodule-update
> make[1]: Nothing to be done for `all'.
> make[1]: *** No rule to make target 
> `/Users/John/Documents/Development/Projects/Qemu/qemu-git/capstone/libcapstone.a'.
>   Stop.
> make: *** [subdir-capstone] Error 2

Apparently the submodule update is not working right for you. What do
you get when you run:

 git submodule update capstone

?

>>> I took a look at the capstone folder. There is no 'make' file in this 
>>> folder. Should there be one?
>>
>> Yes, the capstone folder should be populated automatically. Is it
>> completely empty for you?
> 
> It isn't empty. All I see are two folders: obj and docs.

Maybe try to clean the folder first:

 rm -r capstone
 mkdir capstone
 make git-submodule-update

If that does not help, maybe try a completely fresh git checkout?

 Thomas



[Qemu-devel] [Bug 1828723] [NEW] [RFE] option to suppress gemu_log() output

2019-05-12 Thread Thomas Moschny
Public bug reported:

With user mode emulation, messages from genu_log() are emitted
unconditionally to stderr. I didn't find a way to suppress them. It
would be nice to have options similar to the -D/-d options to be able to
filter and/or redirect the output.

My use case is chroot/container execution for different architectures
via binfmt. In this case it will appear as if the binary in question had
emitted messages like "Unsupported setsockopt ..." to stderr while in
fact the message came from qemu-*-static.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  [RFE] option to suppress gemu_log() output

Status in QEMU:
  New

Bug description:
  With user mode emulation, messages from genu_log() are emitted
  unconditionally to stderr. I didn't find a way to suppress them. It
  would be nice to have options similar to the -D/-d options to be able
  to filter and/or redirect the output.

  My use case is chroot/container execution for different architectures
  via binfmt. In this case it will appear as if the binary in question
  had emitted messages like "Unsupported setsockopt ..." to stderr while
  in fact the message came from qemu-*-static.

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



[Qemu-devel] [PATCH 11/13] target/arm/cpu64: max cpu: Introduce sve-vls-map

2019-05-12 Thread Andrew Jones
Introduce another cpu property to control SVE vector lengths,
sve-vls-map, which allows the user to explicitly select the
set of vector lengths the guest can use. The map must conform
to QEMU's limits and architectural constraints, checked when
the property is set. Inconsistencies with sve-max-vq are also
checked. The bit number of a set bit in the map represents the
allowed vector length in number of quadwords.

Note, as the map is implemented with a single 64-bit word we
currently only support up to 8192-bit vectors. As QEMU and
KVM only support up to 2048-bit vectors then this sufficient
now, and probably for some time. Extending the bitmap beyond
a single word will likely require changing the property to
a string and adding yet another parser to QEMU.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu.c |  4 +++
 target/arm/cpu.h |  3 ++
 target/arm/cpu64.c   | 70 +---
 target/arm/helper.c  | 10 ++-
 target/arm/monitor.c |  9 --
 5 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a181fa8dc1a7..ea0e24bba8b6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1188,6 +1188,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 set_feature(env, ARM_FEATURE_VBAR);
 }
 
+if (!kvm_enabled() && !cpu->sve_vls_map) {
+cpu->sve_vls_map = BIT_MASK(cpu->sve_max_vq) - 1;
+}
+
 register_cp_regs_for_features(cpu);
 arm_cpu_register_gdb_regs_for_features(cpu);
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8292d547e8f9..f0d0ce759ba8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -920,6 +920,9 @@ struct ARMCPU {
 
 /* Used to set the maximum vector length the cpu will support.  */
 uint32_t sve_max_vq;
+
+/* Each bit represents a supported vector length of (bitnum * 16) bytes */
+uint64_t sve_vls_map;
 };
 
 static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 3756e7e2a3e5..9ac702d54136 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -273,14 +273,73 @@ static void cpu_max_set_sve_vq(Object *obj, Visitor *v, 
const char *name,
 
 visit_type_uint32(v, name, >sve_max_vq, );
 
-if (!err && (cpu->sve_max_vq == 0 || cpu->sve_max_vq > ARM_MAX_VQ)) {
-error_setg(, "unsupported SVE vector length");
-error_append_hint(, "Valid sve-max-vq in range [1-%d]\n",
-  ARM_MAX_VQ);
+if (!err) {
+if (cpu->sve_max_vq == 0 || cpu->sve_max_vq > ARM_MAX_VQ) {
+error_setg(, "unsupported SVE vector length");
+error_append_hint(, "Valid sve-max-vq in range [1-%d]\n",
+  ARM_MAX_VQ);
+} else if (cpu->sve_vls_map &&
+   cpu->sve_max_vq != arm_cpu_fls64(cpu->sve_vls_map)) {
+error_setg(, "sve-vls-map and sve-max-vq are inconsistent");
+}
 }
 error_propagate(errp, err);
 }
 
+static void cpu_get_sve_vls_map(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+visit_type_uint64(v, name, >sve_vls_map, errp);
+}
+
+static void cpu_set_sve_vls_map(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+Error *err = NULL;
+uint64_t mask = ~(BIT_MASK(ARM_MAX_VQ - 1) - 1);
+int i;
+
+visit_type_uint64(v, name, >sve_vls_map, errp);
+
+if (!err) {
+if (cpu->sve_vls_map == 0) {
+error_setg(, "SVE vector length map cannot be zero");
+} else if (cpu->sve_vls_map & mask) {
+error_setg(, "SVE vector length map has unsupported lengths");
+error_append_hint(, "Valid vector lengths in range [1-%d]\n",
+  ARM_MAX_VQ);
+} else if (cpu->sve_max_vq != ARM_MAX_VQ &&
+   cpu->sve_max_vq != arm_cpu_fls64(cpu->sve_vls_map)) {
+/*
+ * If the user provides both sve-max-vq and sve-vls-map, with
+ * sve-max-vq first, then, unless ARM_MAX_VQ is selected for
+ * sve-max-vq, we can catch inconsistencies. If ARM_MAX_VQ was
+ * selected then sve-max-vq will get silently overwritten with
+ * the max sve-vls-map provides. This is the best we can do
+ * without initially setting sve-max-vq to -1 like we do for KVM.
+ */
+error_setg(, "sve-vls-map and sve-max-vq are inconsistent");
+} else {
+cpu->sve_max_vq = arm_cpu_fls64(cpu->sve_vls_map);
+mask = 0;
+for (i = 1; i < cpu->sve_max_vq; ++i) {
+if (is_power_of_2(i)) {
+mask |= BIT_MASK(i - 1);
+}
+}
+if ((cpu->sve_vls_map & mask) != mask) {
+error_setg(, "SVE vector 

[Qemu-devel] [PATCH 03/13] HACK: linux header update

2019-05-12 Thread Andrew Jones
---
 linux-headers/asm-arm64/kvm.h | 41 +
 linux-headers/asm-arm64/sve_context.h | 53 +++
 linux-headers/linux/kvm.h |  5 +++
 3 files changed, 99 insertions(+)
 create mode 100644 linux-headers/asm-arm64/sve_context.h

diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index e6a98c14c848..3e73a4c67b67 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define __KVM_HAVE_GUEST_DEBUG
 #define __KVM_HAVE_IRQ_LINE
@@ -102,6 +103,7 @@ struct kvm_regs {
 #define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */
 #define KVM_ARM_VCPU_PSCI_0_2  2 /* CPU uses PSCI v0.2 */
 #define KVM_ARM_VCPU_PMU_V33 /* Support guest PMUv3 */
+#define KVM_ARM_VCPU_SVE   4 /* enable SVE for this CPU */
 
 struct kvm_vcpu_init {
__u32 target;
@@ -226,6 +228,45 @@ struct kvm_vcpu_events {
 KVM_REG_ARM_FW | ((r) & 0x))
 #define KVM_REG_ARM_PSCI_VERSION   KVM_REG_ARM_FW_REG(0)
 
+/* SVE registers */
+#define KVM_REG_ARM64_SVE  (0x15 << KVM_REG_ARM_COPROC_SHIFT)
+
+/* Z- and P-regs occupy blocks at the following offsets within this range: */
+#define KVM_REG_ARM64_SVE_ZREG_BASE0
+#define KVM_REG_ARM64_SVE_PREG_BASE0x400
+#define KVM_REG_ARM64_SVE_FFR_BASE 0x600
+
+#define KVM_ARM64_SVE_NUM_ZREGS__SVE_NUM_ZREGS
+#define KVM_ARM64_SVE_NUM_PREGS__SVE_NUM_PREGS
+
+#define KVM_ARM64_SVE_MAX_SLICES   32
+
+#define KVM_REG_ARM64_SVE_ZREG(n, i)   \
+   (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_ZREG_BASE | \
+KVM_REG_SIZE_U2048 |   \
+(((n) & (KVM_ARM64_SVE_NUM_ZREGS - 1)) << 5) | \
+((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_REG_ARM64_SVE_PREG(n, i)   \
+   (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_PREG_BASE | \
+KVM_REG_SIZE_U256 |\
+(((n) & (KVM_ARM64_SVE_NUM_PREGS - 1)) << 5) | \
+((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_REG_ARM64_SVE_FFR(i)   \
+   (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_FFR_BASE | \
+KVM_REG_SIZE_U256 |\
+((i) & (KVM_ARM64_SVE_MAX_SLICES - 1)))
+
+#define KVM_ARM64_SVE_VQ_MIN __SVE_VQ_MIN
+#define KVM_ARM64_SVE_VQ_MAX __SVE_VQ_MAX
+
+/* Vector lengths pseudo-register: */
+#define KVM_REG_ARM64_SVE_VLS  (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
+KVM_REG_SIZE_U512 | 0x)
+#define KVM_ARM64_SVE_VLS_WORDS\
+   ((KVM_ARM64_SVE_VQ_MAX - KVM_ARM64_SVE_VQ_MIN) / 64 + 1)
+
 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR  0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS 1
diff --git a/linux-headers/asm-arm64/sve_context.h 
b/linux-headers/asm-arm64/sve_context.h
new file mode 100644
index ..1d0e3e1d0950
--- /dev/null
+++ b/linux-headers/asm-arm64/sve_context.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (C) 2017-2018 ARM Limited */
+
+/*
+ * For use by other UAPI headers only.
+ * Do not make direct use of header or its definitions.
+ */
+
+#ifndef __ASM_SVE_CONTEXT_H
+#define __ASM_SVE_CONTEXT_H
+
+#include 
+
+#define __SVE_VQ_BYTES 16  /* number of bytes per quadword */
+
+#define __SVE_VQ_MIN   1
+#define __SVE_VQ_MAX   512
+
+#define __SVE_VL_MIN   (__SVE_VQ_MIN * __SVE_VQ_BYTES)
+#define __SVE_VL_MAX   (__SVE_VQ_MAX * __SVE_VQ_BYTES)
+
+#define __SVE_NUM_ZREGS32
+#define __SVE_NUM_PREGS16
+
+#define __sve_vl_valid(vl) \
+   ((vl) % __SVE_VQ_BYTES == 0 &&  \
+(vl) >= __SVE_VL_MIN &&\
+(vl) <= __SVE_VL_MAX)
+
+#define __sve_vq_from_vl(vl)   ((vl) / __SVE_VQ_BYTES)
+#define __sve_vl_from_vq(vq)   ((vq) * __SVE_VQ_BYTES)
+
+#define __SVE_ZREG_SIZE(vq)((__u32)(vq) * __SVE_VQ_BYTES)
+#define __SVE_PREG_SIZE(vq)((__u32)(vq) * (__SVE_VQ_BYTES / 8))
+#define __SVE_FFR_SIZE(vq) __SVE_PREG_SIZE(vq)
+
+#define __SVE_ZREGS_OFFSET 0
+#define __SVE_ZREG_OFFSET(vq, n) \
+   (__SVE_ZREGS_OFFSET + __SVE_ZREG_SIZE(vq) * (n))
+#define __SVE_ZREGS_SIZE(vq) \
+   (__SVE_ZREG_OFFSET(vq, __SVE_NUM_ZREGS) - __SVE_ZREGS_OFFSET)
+
+#define __SVE_PREGS_OFFSET(vq) \
+   (__SVE_ZREGS_OFFSET + __SVE_ZREGS_SIZE(vq))
+#define __SVE_PREG_OFFSET(vq, n) \
+   (__SVE_PREGS_OFFSET(vq) + __SVE_PREG_SIZE(vq) * (n))
+#define __SVE_PREGS_SIZE(vq) \
+   (__SVE_PREG_OFFSET(vq, __SVE_NUM_PREGS) - __SVE_PREGS_OFFSET(vq))
+

[Qemu-devel] [PATCH 04/13] target/arm/kvm: Move the get/put of fpsimd registers out

2019-05-12 Thread Andrew Jones
Move the getting/putting of the fpsimd registers out of
kvm_arch_get/put_registers() into their own helper functions
to prepare for alternatively getting/putting SVE registers.

No functional change.

Signed-off-by: Andrew Jones 
---
 target/arm/kvm64.c | 148 +++--
 1 file changed, 88 insertions(+), 60 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index ba232b27a6d3..61947f3716e1 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -706,13 +706,53 @@ int kvm_arm_cpreg_level(uint64_t regidx)
 #define AARCH64_SIMD_CTRL_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U32 | \
  KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
 
+static int kvm_arch_put_fpsimd(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+struct kvm_one_reg reg;
+uint32_t fpr;
+int i, ret;
+
+for (i = 0; i < 32; i++) {
+uint64_t *q = aa64_vfp_qreg(env, i);
+#ifdef HOST_WORDS_BIGENDIAN
+uint64_t fp_val[2] = { q[1], q[0] };
+reg.addr = (uintptr_t)fp_val;
+#else
+reg.addr = (uintptr_t)q;
+#endif
+reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+}
+
+reg.addr = (uintptr_t)();
+fpr = vfp_get_fpsr(env);
+reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+
+reg.addr = (uintptr_t)();
+fpr = vfp_get_fpcr(env);
+reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+
+return 0;
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
 struct kvm_one_reg reg;
-uint32_t fpr;
 uint64_t val;
-int i;
-int ret;
+int i, ret;
 unsigned int el;
 
 ARMCPU *cpu = ARM_CPU(cs);
@@ -802,33 +842,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 }
 }
 
-/* Advanced SIMD and FP registers. */
-for (i = 0; i < 32; i++) {
-uint64_t *q = aa64_vfp_qreg(env, i);
-#ifdef HOST_WORDS_BIGENDIAN
-uint64_t fp_val[2] = { q[1], q[0] };
-reg.addr = (uintptr_t)fp_val;
-#else
-reg.addr = (uintptr_t)q;
-#endif
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
-if (ret) {
-return ret;
-}
-}
-
-reg.addr = (uintptr_t)();
-fpr = vfp_get_fpsr(env);
-reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
-if (ret) {
-return ret;
-}
-
-fpr = vfp_get_fpcr(env);
-reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_arch_put_fpsimd(cs);
 if (ret) {
 return ret;
 }
@@ -849,14 +863,54 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 return ret;
 }
 
+static int kvm_arch_get_fpsimd(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+struct kvm_one_reg reg;
+uint32_t fpr;
+int i, ret;
+
+for (i = 0; i < 32; i++) {
+uint64_t *q = aa64_vfp_qreg(env, i);
+reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
+reg.addr = (uintptr_t)q;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+} else {
+#ifdef HOST_WORDS_BIGENDIAN
+uint64_t t;
+t = q[0], q[0] = q[1], q[1] = t;
+#endif
+}
+}
+
+reg.addr = (uintptr_t)();
+reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+}
+vfp_set_fpsr(env, fpr);
+
+reg.addr = (uintptr_t)();
+reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpcr);
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+}
+vfp_set_fpcr(env, fpr);
+
+return 0;
+}
+
 int kvm_arch_get_registers(CPUState *cs)
 {
 struct kvm_one_reg reg;
 uint64_t val;
-uint32_t fpr;
 unsigned int el;
-int i;
-int ret;
+int i, ret;
 
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = >env;
@@ -945,36 +999,10 @@ int kvm_arch_get_registers(CPUState *cs)
 env->spsr = env->banked_spsr[i];
 }
 
-/* Advanced SIMD and FP registers */
-for (i = 0; i < 32; i++) {
-uint64_t *q = aa64_vfp_qreg(env, i);
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-reg.addr = (uintptr_t)q;
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
-if (ret) {
-return ret;
-} else {
-#ifdef HOST_WORDS_BIGENDIAN
-uint64_t t;
-t = q[0], q[0] = q[1], q[1] = t;
-#endif
-}
-}
-
-reg.addr = (uintptr_t)();
-reg.id = AARCH64_SIMD_CTRL_REG(fp_regs.fpsr);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, 

[Qemu-devel] [PATCH 05/13] target/arm/kvm: Add kvm_arch_get/put_sve

2019-05-12 Thread Andrew Jones
These are the SVE equivalents to kvm_arch_get/put_fpsimd.

Signed-off-by: Andrew Jones 
---
 target/arm/kvm64.c | 127 +++--
 1 file changed, 123 insertions(+), 4 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 61947f3716e1..86362f4cd7d0 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -658,11 +658,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
 {
 /* Return true if the regidx is a register we should synchronize
- * via the cpreg_tuples array (ie is not a core reg we sync by
- * hand in kvm_arch_get/put_registers())
+ * via the cpreg_tuples array (ie is not a core or sve reg that
+ * we sync by hand in kvm_arch_get/put_registers())
  */
 switch (regidx & KVM_REG_ARM_COPROC_MASK) {
 case KVM_REG_ARM_CORE:
+case KVM_REG_ARM64_SVE:
 return false;
 default:
 return true;
@@ -748,6 +749,61 @@ static int kvm_arch_put_fpsimd(CPUState *cs)
 return 0;
 }
 
+static int kvm_arch_put_sve(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+struct kvm_one_reg reg;
+int n, ret;
+
+for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; n++) {
+uint64_t *q = aa64_vfp_qreg(env, n);
+#ifdef HOST_WORDS_BIGENDIAN
+uint64_t d[ARM_MAX_VQ * 2];
+int i;
+for (i = 0; i < cpu->sve_max_vq * 2; i++) {
+d[i] = q[cpu->sve_max_vq * 2 - 1 - i];
+}
+reg.addr = (uintptr_t)d;
+#else
+reg.addr = (uintptr_t)q;
+#endif
+reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+}
+
+for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; n++) {
+uint64_t *p = >vfp.pregs[n].p[0];
+#ifdef HOST_WORDS_BIGENDIAN
+uint64_t d[ARM_MAX_VQ * 2];
+int i;
+for (i = 0; i < cpu->sve_max_vq * 2 / 8; i++) {
+d[i] = p[cpu->sve_max_vq * 2 / 8 - 1 - i];
+}
+reg.addr = (uintptr_t)d;
+#else
+reg.addr = (uintptr_t)p;
+#endif
+reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+}
+
+reg.addr = (uintptr_t)>vfp.pregs[FFR_PRED_NUM].p[0];
+reg.id = KVM_REG_ARM64_SVE_FFR(0);
+ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+if (ret) {
+return ret;
+}
+
+return 0;
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
 struct kvm_one_reg reg;
@@ -842,7 +898,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 }
 }
 
-ret = kvm_arch_put_fpsimd(cs);
+if (!cpu_isar_feature(aa64_sve, cpu)) {
+ret = kvm_arch_put_fpsimd(cs);
+} else {
+ret = kvm_arch_put_sve(cs);
+}
 if (ret) {
 return ret;
 }
@@ -905,6 +965,61 @@ static int kvm_arch_get_fpsimd(CPUState *cs)
 return 0;
 }
 
+static int kvm_arch_get_sve(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+struct kvm_one_reg reg;
+int n, ret;
+
+for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; n++) {
+uint64_t *q = aa64_vfp_qreg(env, n);
+reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
+reg.addr = (uintptr_t)q;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+} else {
+#ifdef HOST_WORDS_BIGENDIAN
+int i = 0, j = cpu->sve_max_vq * 2 - 1;
+while (i < j) {
+uint64_t t;
+t = q[i], q[i] = q[j], q[j] = t;
+++i, --j;
+}
+#endif
+}
+}
+
+for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; n++) {
+uint64_t *p = >vfp.pregs[n].p[0];
+reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
+reg.addr = (uintptr_t)p;
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+} else {
+#ifdef HOST_WORDS_BIGENDIAN
+int i = 0, j = cpu->sve_max_vq * 2 / 8 - 1;
+while (i < j) {
+uint64_t t;
+t = q[i], q[i] = q[j], q[j] = t;
+++i, --j;
+}
+#endif
+}
+}
+
+reg.addr = (uintptr_t)>vfp.pregs[FFR_PRED_NUM].p[0];
+reg.id = KVM_REG_ARM64_SVE_FFR(0);
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+}
+
+return 0;
+}
+
 int kvm_arch_get_registers(CPUState *cs)
 {
 struct kvm_one_reg reg;
@@ -999,7 +1114,11 @@ int kvm_arch_get_registers(CPUState *cs)
 env->spsr = env->banked_spsr[i];
 }
 
-ret = kvm_arch_get_fpsimd(cs);
+if (!cpu_isar_feature(aa64_sve, cpu)) {
+ret = kvm_arch_get_fpsimd(cs);
+} else {
+ret = kvm_arch_get_sve(cs);
+}
 if (ret) {
 return ret;
 }
-- 
2.20.1




[Qemu-devel] [PATCH 08/13] target/arm/monitor: Add query-sve-vector-lengths

2019-05-12 Thread Andrew Jones
Provide a QMP interface to query the supported SVE vector lengths.
A migratable guest will need to explicitly specify a valid set of
lengths on the command line and that set can be obtained from the
list returned with this QMP command.

This patch only introduces the QMP command with the TCG implementation.
The result may not yet be correct for KVM. Following patches ensure
the KVM result is correct.

Signed-off-by: Andrew Jones 
---
 qapi/target.json | 34 
 target/arm/monitor.c | 62 
 tests/qmp-cmd-test.c |  1 +
 3 files changed, 97 insertions(+)

diff --git a/qapi/target.json b/qapi/target.json
index 1d4d54b6002e..ca1e85254780 100644
--- a/qapi/target.json
+++ b/qapi/target.json
@@ -397,6 +397,40 @@
 { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'],
   'if': 'defined(TARGET_ARM)' }
 
+##
+# @SVEVectorLengths:
+#
+# The struct contains a list of integers where each integer is a valid
+# SVE vector length for a KVM guest on this host. The vector lengths
+# are in quadword (128-bit) units, e.g. '4' means 512 bits (64 bytes).
+#
+# @vls:  list of vector lengths in quadwords.
+#
+# Since: 4.1
+##
+{ 'struct': 'SVEVectorLengths',
+  'data': { 'vls': ['int'] },
+  'if': 'defined(TARGET_ARM)' }
+
+##
+# @query-sve-vector-lengths:
+#
+# This command is ARM-only. It will return a list of SVEVectorLengths
+# objects. The list describes all valid SVE vector length sets.
+#
+# Returns: a list of SVEVectorLengths objects
+#
+# Since: 4.1
+#
+# -> { "execute": "query-sve-vector-lengths" }
+# <- { "return": [ { "vls": [ 1 ] },
+#  { "vls": [ 1, 2 ] },
+#  { "vls": [ 1, 2, 4 ] } ] }
+#
+##
+{ 'command': 'query-sve-vector-lengths', 'returns': ['SVEVectorLengths'],
+  'if': 'defined(TARGET_ARM)' }
+
 ##
 # @CpuModelExpansionInfo:
 #
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 41b32b94b258..8b2afa255c92 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -24,6 +24,7 @@
 #include "hw/boards.h"
 #include "kvm_arm.h"
 #include "qapi/qapi-commands-target.h"
+#include "monitor/hmp-target.h"
 
 static GICCapability *gic_cap_new(int version)
 {
@@ -82,3 +83,64 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
 
 return head;
 }
+
+static SVEVectorLengths *qmp_sve_vls_get(void)
+{
+CPUArchState *env = mon_get_cpu_env();
+ARMCPU *cpu = arm_env_get_cpu(env);
+SVEVectorLengths *vls = g_new(SVEVectorLengths, 1);
+intList **v = >vls;
+int i;
+
+if (cpu->sve_max_vq == 0) {
+*v = g_new0(intList, 1); /* one vl of 0 means none supported */
+return vls;
+}
+
+for (i = 1; i <= cpu->sve_max_vq; ++i) {
+*v = g_new0(intList, 1);
+(*v)->value = i;
+v = &(*v)->next;
+}
+
+return vls;
+}
+
+static SVEVectorLengths *qmp_sve_vls_dup_and_truncate(SVEVectorLengths *vls)
+{
+SVEVectorLengths *trunc_vls;
+intList **v, *p = vls->vls;
+
+if (!p->next) {
+return NULL;
+}
+
+trunc_vls = g_new(SVEVectorLengths, 1);
+v = _vls->vls;
+
+for (; p->next; p = p->next) {
+*v = g_new0(intList, 1);
+(*v)->value = p->value;
+v = &(*v)->next;
+}
+
+return trunc_vls;
+}
+
+SVEVectorLengthsList *qmp_query_sve_vector_lengths(Error **errp)
+{
+SVEVectorLengthsList *vls_list = g_new0(SVEVectorLengthsList, 1);
+SVEVectorLengths *vls = qmp_sve_vls_get();
+
+while (vls) {
+vls_list->value = vls;
+vls = qmp_sve_vls_dup_and_truncate(vls);
+if (vls) {
+SVEVectorLengthsList *next = vls_list;
+vls_list = g_new0(SVEVectorLengthsList, 1);
+vls_list->next = next;
+}
+}
+
+return vls_list;
+}
diff --git a/tests/qmp-cmd-test.c b/tests/qmp-cmd-test.c
index 9f5228cd9951..3d714dbc6a4a 100644
--- a/tests/qmp-cmd-test.c
+++ b/tests/qmp-cmd-test.c
@@ -90,6 +90,7 @@ static bool query_is_blacklisted(const char *cmd)
 /* Success depends on target arch: */
 "query-cpu-definitions",  /* arm, i386, ppc, s390x */
 "query-gic-capabilities", /* arm */
+"query-sve-vector-lengths", /* arm */
 /* Success depends on target-specific build configuration: */
 "query-pci",  /* CONFIG_PCI */
 /* Success depends on launching SEV guest */
-- 
2.20.1




[Qemu-devel] [PATCH 10/13] target/arm/monitor: kvm: only return valid sve vector sets

2019-05-12 Thread Andrew Jones
While the TCG SVE implementation can implement all vector lengths
which are a quadword multiple, up to some maximum length, KVM can
only provide what the host supports, and not all multiples are
required to be supported by the architecture. With this patch
we extend the QMP query to ask KVM for the valid vectors when KVM
is enabled.

Signed-off-by: Andrew Jones 
---
 target/arm/monitor.c | 43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 8b2afa255c92..3e13dd7c7b7a 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -84,6 +84,41 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
 return head;
 }
 
+#ifdef CONFIG_KVM
+static SVEVectorLengths *qmp_kvm_sve_vls_get(void)
+{
+CPUArchState *env = mon_get_cpu_env();
+ARMCPU *cpu = arm_env_get_cpu(env);
+uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS];
+SVEVectorLengths *vls = g_new(SVEVectorLengths, 1);
+intList **v = >vls;
+int ret, i;
+
+ret = kvm_arm_get_sve_vls(CPU(cpu), sve_vls);
+if (ret <= 0) {
+*v = g_new0(intList, 1); /* one vl of 0 means none supported */
+return vls;
+}
+
+for (i = KVM_ARM64_SVE_VQ_MIN; i <= ret; ++i) {
+int bitval = (sve_vls[(i - KVM_ARM64_SVE_VQ_MIN) / 64] >>
+  ((i - KVM_ARM64_SVE_VQ_MIN) % 64)) & 1;
+if (bitval) {
+*v = g_new0(intList, 1);
+(*v)->value = i;
+v = &(*v)->next;
+}
+}
+
+return vls;
+}
+#else
+static SVEVectorLengths *qmp_kvm_sve_vls_get(void)
+{
+return NULL;
+}
+#endif
+
 static SVEVectorLengths *qmp_sve_vls_get(void)
 {
 CPUArchState *env = mon_get_cpu_env();
@@ -130,7 +165,13 @@ static SVEVectorLengths 
*qmp_sve_vls_dup_and_truncate(SVEVectorLengths *vls)
 SVEVectorLengthsList *qmp_query_sve_vector_lengths(Error **errp)
 {
 SVEVectorLengthsList *vls_list = g_new0(SVEVectorLengthsList, 1);
-SVEVectorLengths *vls = qmp_sve_vls_get();
+SVEVectorLengths *vls;
+
+if (kvm_enabled()) {
+vls = qmp_kvm_sve_vls_get();
+} else {
+vls = qmp_sve_vls_get();
+}
 
 while (vls) {
 vls_list->value = vls;
-- 
2.20.1




[Qemu-devel] [PATCH 02/13] update-linux-headers: Add sve_context.h to asm-arm64

2019-05-12 Thread Andrew Jones
Signed-off-by: Andrew Jones 
---
 scripts/update-linux-headers.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index c3819d2b983d..e1fce54f8aa3 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -99,6 +99,9 @@ for arch in $ARCHLIST; do
 cp "$tmpdir/include/asm/$header" "$output/linux-headers/asm-$arch"
 done
 
+if [ $arch = arm64 ]; then
+cp "$tmpdir/include/asm/sve_context.h" 
"$output/linux-headers/asm-arm64/"
+fi
 if [ $arch = mips ]; then
 cp "$tmpdir/include/asm/sgidefs.h" "$output/linux-headers/asm-mips/"
 cp "$tmpdir/include/asm/unistd_o32.h" "$output/linux-headers/asm-mips/"
-- 
2.20.1




[Qemu-devel] [PATCH 12/13] target/arm/kvm: max cpu: Add support for sve-vls-map

2019-05-12 Thread Andrew Jones
The max cpu type can have its SVE vector lengths explicitly set
with the sve-vls-map property. This patch allows that property
to work when KVM is in use. The map must conform to additional
constraints for KVM which are checked at vcpu init.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu64.c |  7 +++---
 target/arm/kvm64.c | 56 +++---
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 9ac702d54136..94f3dd5b51e5 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,7 +310,7 @@ static void cpu_set_sve_vls_map(Object *obj, Visitor *v, 
const char *name,
 error_setg(, "SVE vector length map has unsupported lengths");
 error_append_hint(, "Valid vector lengths in range [1-%d]\n",
   ARM_MAX_VQ);
-} else if (cpu->sve_max_vq != ARM_MAX_VQ &&
+} else if (cpu->sve_max_vq != ARM_MAX_VQ && cpu->sve_max_vq != -1 &&
cpu->sve_max_vq != arm_cpu_fls64(cpu->sve_vls_map)) {
 /*
  * If the user provides both sve-max-vq and sve-vls-map, with
@@ -433,13 +433,12 @@ static void aarch64_max_initfn(Object *obj)
 #endif
 
 cpu->sve_max_vq = ARM_MAX_VQ;
-
-object_property_add(obj, "sve-vls-map", "uint64", cpu_get_sve_vls_map,
-cpu_set_sve_vls_map, NULL, NULL, _fatal);
 }
 
 object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
 cpu_max_set_sve_vq, NULL, NULL, _fatal);
+object_property_add(obj, "sve-vls-map", "uint64", cpu_get_sve_vls_map,
+cpu_set_sve_vls_map, NULL, NULL, _fatal);
 }
 
 struct ARMCPUInfo {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 11c6334a7c08..5506f019c190 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -685,9 +685,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
 } else {
 unset_feature(>features, ARM_FEATURE_PMU);
 }
-if (cpu->sve_max_vq) {
+if (cpu->sve_max_vq || cpu->sve_vls_map) {
 if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SVE)) {
-if (cpu->sve_max_vq == -1) {
+if (cpu->sve_max_vq == -1 && !cpu->sve_vls_map) {
 cpu->sve_max_vq = 0;
 } else {
 error_report("This KVM host does not support SVE");
@@ -704,12 +704,62 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-if (cpu->sve_max_vq) {
+if (cpu->sve_max_vq || cpu->sve_vls_map) {
 uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS];
 ret = kvm_arm_get_sve_vls(cs, sve_vls);
 if (ret < 0) {
 return ret;
 }
+if (cpu->sve_vls_map) {
+uint64_t ovls;
+int i;
+
+/*
+ * We currently only support a single VLS word, as that should
+ * be sufficient for some time (vq=64 means a 8192-bit vector
+ * and KVM currently only supports up to 2048-bit vectors).
+ * The choice to only support a single word for now is due to
+ * the need to input it on the command line. It's much simpler
+ * to input a word as a cpu property than an array of words.
+ * So for now just warn if we detect our assumption was wrong.
+ */
+for (i = 1; i < KVM_ARM64_SVE_VLS_WORDS; ++i) {
+if (sve_vls[i]) {
+warn_report("KVM supports vector lengths larger than "
+"sve-vls-map can select");
+sve_vls[i] = 0;
+}
+}
+
+ovls = sve_vls[0];
+sve_vls[0] = cpu->sve_vls_map;
+
+if (cpu->sve_vls_map & ~ovls) {
+error_report("sve-vls-map=0x%lx is not valid on this host "
+ "which supports 0x%lx", cpu->sve_vls_map, ovls);
+return -EINVAL;
+}
+
+i = arm_cpu_fls64(cpu->sve_vls_map);
+if (cpu->sve_max_vq && cpu->sve_max_vq != -1 &&
+cpu->sve_max_vq != i) {
+error_report("sve-vls-map and sve-max-vq are inconsistent");
+return -EINVAL;
+}
+cpu->sve_max_vq = i;
+
+/*
+ * sve-vls-map must have all the same vector lengths up to its
+ * max vq that the host supports.
+ */
+if (cpu->sve_vls_map != (ovls & (BIT_MASK(cpu->sve_max_vq) - 1))) {
+error_report("sve-vls-map=0x%lx is not valid on this host "
+ "which supports 0x%lx", cpu->sve_vls_map, ovls);
+error_printf("All host vector lengths up to %d must also "
+ "be selected.\n", cpu->sve_max_vq);
+return -EINVAL;
+}
+}
 if (cpu->sve_max_vq == -1) {
 cpu->sve_max_vq = ret;
 } else 

[Qemu-devel] [PATCH 13/13] target/arm/kvm: host cpu: Add support for sve-vls-map

2019-05-12 Thread Andrew Jones
Allow the host cpu type to enable SVE in guests with the sve-vls-map
cpu property.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu.c   |  1 +
 target/arm/cpu.h   |  2 ++
 target/arm/cpu64.c | 12 +---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ea0e24bba8b6..a5c01ff42c78 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -,6 +,7 @@ static void arm_host_initfn(Object *obj)
 ARMCPU *cpu = ARM_CPU(obj);
 
 kvm_arm_set_cpu_features_from_host(cpu);
+aarch64_add_sve_vls_map_property(obj);
 arm_cpu_post_init(obj);
 }
 
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f0d0ce759ba8..13731ccb39f3 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -976,11 +976,13 @@ int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t 
*buf, int reg);
 void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
 void aarch64_sve_change_el(CPUARMState *env, int old_el,
int new_el, bool el0_a64);
+void aarch64_add_sve_vls_map_property(Object *obj);
 #else
 static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { }
 static inline void aarch64_sve_change_el(CPUARMState *env, int o,
  int n, bool a)
 { }
+void aarch64_add_sve_vls_map_property(Object *obj) { }
 #endif
 
 target_ulong do_arm_semihosting(CPUARMState *env);
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 94f3dd5b51e5..3b0b900a4d97 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -310,7 +310,8 @@ static void cpu_set_sve_vls_map(Object *obj, Visitor *v, 
const char *name,
 error_setg(, "SVE vector length map has unsupported lengths");
 error_append_hint(, "Valid vector lengths in range [1-%d]\n",
   ARM_MAX_VQ);
-} else if (cpu->sve_max_vq != ARM_MAX_VQ && cpu->sve_max_vq != -1 &&
+} else if (cpu->sve_max_vq && cpu->sve_max_vq != ARM_MAX_VQ &&
+   cpu->sve_max_vq != -1 &&
cpu->sve_max_vq != arm_cpu_fls64(cpu->sve_vls_map)) {
 /*
  * If the user provides both sve-max-vq and sve-vls-map, with
@@ -340,6 +341,12 @@ static void cpu_set_sve_vls_map(Object *obj, Visitor *v, 
const char *name,
 error_propagate(errp, err);
 }
 
+void aarch64_add_sve_vls_map_property(Object *obj)
+{
+object_property_add(obj, "sve-vls-map", "uint64", cpu_get_sve_vls_map,
+cpu_set_sve_vls_map, NULL, NULL, _fatal);
+}
+
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
  * otherwise, a CPU with as many features enabled as our emulation supports.
  * The version of '-cpu max' for qemu-system-arm is defined in cpu.c;
@@ -437,8 +444,7 @@ static void aarch64_max_initfn(Object *obj)
 
 object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
 cpu_max_set_sve_vq, NULL, NULL, _fatal);
-object_property_add(obj, "sve-vls-map", "uint64", cpu_get_sve_vls_map,
-cpu_set_sve_vls_map, NULL, NULL, _fatal);
+aarch64_add_sve_vls_map_property(obj);
 }
 
 struct ARMCPUInfo {
-- 
2.20.1




[Qemu-devel] [PATCH 06/13] target/arm/kvm: max cpu: Enable SVE when available

2019-05-12 Thread Andrew Jones
Enable SVE in the KVM guest when the 'max' cpu type is configured
and KVM supports it. KVM SVE requires use of the new finalize
vcpu ioctl, so we add that now too.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu64.c   |  1 +
 target/arm/kvm.c |  5 +
 target/arm/kvm64.c   | 16 +++-
 target/arm/kvm_arm.h | 12 
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 228906f26786..6c19ef6837d5 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -292,6 +292,7 @@ static void aarch64_max_initfn(Object *obj)
 
 if (kvm_enabled()) {
 kvm_arm_set_cpu_features_from_host(cpu);
+cpu->sve_max_vq = ARM_MAX_VQ;
 } else {
 uint64_t t;
 uint32_t u;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 599563461264..c51db4229d0f 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -50,6 +50,11 @@ int kvm_arm_vcpu_init(CPUState *cs)
 return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, );
 }
 
+int kvm_arm_vcpu_finalize(CPUState *cs, int feature)
+{
+return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_FINALIZE, );
+}
+
 void kvm_arm_init_serror_injection(CPUState *cs)
 {
 cap_has_inject_serror_esr = kvm_check_extension(cs->kvm_state,
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 86362f4cd7d0..c2d92df75353 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -622,13 +622,20 @@ int kvm_arch_init_vcpu(CPUState *cs)
 cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
 }
 if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
-cpu->has_pmu = false;
+cpu->has_pmu = false;
 }
 if (cpu->has_pmu) {
 cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
 } else {
 unset_feature(>features, ARM_FEATURE_PMU);
 }
+if (cpu->sve_max_vq) {
+if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SVE)) {
+cpu->sve_max_vq = 0;
+} else {
+cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
+}
+}
 
 /* Do KVM_ARM_VCPU_INIT ioctl */
 ret = kvm_arm_vcpu_init(cs);
@@ -636,6 +643,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
+if (cpu->sve_max_vq) {
+ret = kvm_arm_vcpu_finalize(cs, KVM_ARM_VCPU_SVE);
+if (ret) {
+return ret;
+}
+}
+
 /*
  * When KVM is in use, PSCI is emulated in-kernel and not by qemu.
  * Currently KVM has its own idea about MPIDR assignment, so we
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 2a07333c615f..c488ec3ab410 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -27,6 +27,18 @@
  */
 int kvm_arm_vcpu_init(CPUState *cs);
 
+/**
+ * kvm_arm_vcpu_finalize
+ * @cs: CPUState
+ * @feature: int
+ *
+ * Finalizes the configuration of the specified VCPU feature
+ * by invoking the KVM_ARM_VCPU_FINALIZE ioctl.
+ *
+ * Returns: 0 if success else < 0 error code
+ */
+int kvm_arm_vcpu_finalize(CPUState *cs, int feature);
+
 /**
  * kvm_arm_register_device:
  * @mr: memory region for this device
-- 
2.20.1




[Qemu-devel] [PATCH 01/13] target/arm/kvm64: fix error returns

2019-05-12 Thread Andrew Jones
A couple return -EINVAL's forget their '-'s.

Signed-off-by: Andrew Jones 
---
 target/arm/kvm64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index e3ba1492482f..ba232b27a6d3 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -841,7 +841,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 write_cpustate_to_list(cpu, true);
 
 if (!write_list_to_kvmstate(cpu, level)) {
-return EINVAL;
+return -EINVAL;
 }
 
 kvm_arm_sync_mpstate_to_kvm(cpu);
@@ -982,7 +982,7 @@ int kvm_arch_get_registers(CPUState *cs)
 }
 
 if (!write_kvmstate_to_list(cpu)) {
-return EINVAL;
+return -EINVAL;
 }
 /* Note that it's OK to have registers which aren't in CPUState,
  * so we can ignore a failure return here.
-- 
2.20.1




[Qemu-devel] [PATCH 09/13] target/arm/kvm: Export kvm_arm_get_sve_vls

2019-05-12 Thread Andrew Jones
Signed-off-by: Andrew Jones 
---
 target/arm/kvm64.c   |  7 +--
 target/arm/kvm_arm.h | 20 
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 0c666e405357..11c6334a7c08 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -446,7 +446,8 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
 }
 }
 
-static int kvm_arm_get_sve_vls(CPUState *cs, uint64_t sve_vls[])
+int kvm_arm_get_sve_vls(CPUState *cs,
+uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS])
 {
 struct kvm_one_reg reg = {
 .id = KVM_REG_ARM64_SVE_VLS,
@@ -470,7 +471,9 @@ static int kvm_arm_get_sve_vls(CPUState *cs, uint64_t 
sve_vls[])
 return ret;
 }
 
-static int kvm_arm_set_sve_vls(CPUState *cs, uint64_t sve_vls[], int max_vq)
+static int kvm_arm_set_sve_vls(CPUState *cs,
+   uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS],
+   int max_vq)
 {
 struct kvm_one_reg reg = {
 .id = KVM_REG_ARM64_SVE_VLS,
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index c488ec3ab410..748ed8d54985 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -248,6 +248,26 @@ int kvm_arm_vgic_probe(void);
 void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
 void kvm_arm_pmu_init(CPUState *cs);
 
+/**
+ * kvm_arm_get_sve_vls
+ * @cs: CPUState
+ * @sve_vls: valid vector length bitmap
+ *
+ * Get the valid vector length bitmap. If a bit 'bit' is set
+ * then the host supports a vector length of (bit * 16) bytes.
+ *
+ * For example, if
+ *
+ *   sve_vls[0] = 0xb and
+ *   sve_vls[1 ... KVM_ARM64_SVE_VLS_WORDS-1] = 0,
+ *
+ * then the host supports 16, 32, and 64 byte vector lengths.
+ *
+ * Returns: the highest set bit if successful else < 0 error code
+ */
+int kvm_arm_get_sve_vls(CPUState *cs,
+uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS]);
+
 #else
 
 static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu)
-- 
2.20.1




[Qemu-devel] [PATCH 07/13] target/arm/kvm: max cpu: Allow sve max vector length setting

2019-05-12 Thread Andrew Jones
Allow the cpu type 'max' sve-max-vq property to work with kvm
too. If the property is not specified then the maximum kvm
supports is used. If it is specified we check that kvm supports
that exact length or error out if it doesn't.

Signed-off-by: Andrew Jones 
---
 target/arm/cpu.h   |  4 +++
 target/arm/cpu64.c |  7 ++--
 target/arm/kvm64.c | 80 --
 3 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 733b840a7127..8292d547e8f9 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3122,6 +3122,10 @@ static inline uint64_t arm_sctlr(CPUARMState *env, int 
el)
 }
 }
 
+static inline int arm_cpu_fls64(uint64_t v)
+{
+return !v ? 0 : 64 - clz64(v);
+}
 
 /* Return true if the processor is in big-endian mode. */
 static inline bool arm_cpu_data_is_big_endian(CPUARMState *env)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 6c19ef6837d5..3756e7e2a3e5 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -292,7 +292,7 @@ static void aarch64_max_initfn(Object *obj)
 
 if (kvm_enabled()) {
 kvm_arm_set_cpu_features_from_host(cpu);
-cpu->sve_max_vq = ARM_MAX_VQ;
+cpu->sve_max_vq = -1; /* set in kvm_arch_init_vcpu() */
 } else {
 uint64_t t;
 uint32_t u;
@@ -374,9 +374,10 @@ static void aarch64_max_initfn(Object *obj)
 #endif
 
 cpu->sve_max_vq = ARM_MAX_VQ;
-object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
-cpu_max_set_sve_vq, NULL, NULL, _fatal);
 }
+
+object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
+cpu_max_set_sve_vq, NULL, NULL, _fatal);
 }
 
 struct ARMCPUInfo {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index c2d92df75353..0c666e405357 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -446,6 +446,59 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
 }
 }
 
+static int kvm_arm_get_sve_vls(CPUState *cs, uint64_t sve_vls[])
+{
+struct kvm_one_reg reg = {
+.id = KVM_REG_ARM64_SVE_VLS,
+.addr = (uint64_t)_vls[0],
+};
+int i, ret;
+
+ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+if (ret) {
+return ret;
+}
+
+ret = 0;
+for (i = KVM_ARM64_SVE_VLS_WORDS - 1; i >= 0; --i) {
+if (sve_vls[i]) {
+ret = arm_cpu_fls64(sve_vls[i]) + i * 64;
+break;
+}
+}
+
+return ret;
+}
+
+static int kvm_arm_set_sve_vls(CPUState *cs, uint64_t sve_vls[], int max_vq)
+{
+struct kvm_one_reg reg = {
+.id = KVM_REG_ARM64_SVE_VLS,
+.addr = (uint64_t)_vls[0],
+};
+int i;
+
+for (i = KVM_ARM64_SVE_VLS_WORDS - 1; i >= 0; --i) {
+if (sve_vls[i]) {
+int vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
+while (vq > max_vq) {
+sve_vls[i] &= ~BIT_MASK(vq - 1);
+vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
+}
+if (vq < max_vq) {
+error_report("sve-max-vq=%d is not a valid length", max_vq);
+error_printf("next lowest is %d\n", vq);
+return -EINVAL;
+}
+if (vq == max_vq) {
+break;
+}
+}
+}
+
+return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+}
+
 static inline void set_feature(uint64_t *features, int feature)
 {
 *features |= 1ULL << feature;
@@ -605,7 +658,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
 if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
 !object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU)) {
-fprintf(stderr, "KVM is not supported for this guest CPU type\n");
+error_report("KVM is not supported for this guest CPU type");
 return -EINVAL;
 }
 
@@ -631,7 +684,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 if (cpu->sve_max_vq) {
 if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_SVE)) {
-cpu->sve_max_vq = 0;
+if (cpu->sve_max_vq == -1) {
+cpu->sve_max_vq = 0;
+} else {
+error_report("This KVM host does not support SVE");
+return -EINVAL;
+}
 } else {
 cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
 }
@@ -644,6 +702,24 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 
 if (cpu->sve_max_vq) {
+uint64_t sve_vls[KVM_ARM64_SVE_VLS_WORDS];
+ret = kvm_arm_get_sve_vls(cs, sve_vls);
+if (ret < 0) {
+return ret;
+}
+if (cpu->sve_max_vq == -1) {
+cpu->sve_max_vq = ret;
+} else if (cpu->sve_max_vq > ret) {
+error_report("This KVM host does not support SVE vectors "
+ "of length %d quadwords (%d bytes)",
+ cpu->sve_max_vq, cpu->sve_max_vq * 16);
+return -EINVAL;
+} else {
+ret = 

[Qemu-devel] [PATCH 00/13] target/arm/kvm: enable SVE in guests

2019-05-12 Thread Andrew Jones
With the recent KVM guest SVE support pull request [1] KVM will be
ready for guests with SVE. This series provides the QEMU bits for
that enablement. The series starts with the bits needed for the KVM
SVE ioctls. Then it enables the arm 'max'cpu type, which with TCG
already supports SVE, to also support SVE when using KVM. Next
a new QMP query is added that allows users to ask which vector
lengths are supported by the host, allowing them to select a valid
set of vectors for the guest. In order to select those vectors a
new property 'sve-vls-map' is added to the 'max' cpu type, and then
also to the 'host' cpu type. The table below shows the resulting user
interfaces.

   CPU type | accel | sve-max-vq | sve-vls-map
   ---
 1) max | tcg   |  $MAX_VQ   |  $VLS_MAP
 2) max | kvm   |  $MAX_VQ   |  $VLS_MAP
 3)host | kvm   |  N/A   |  $VLS_MAP

Where for (1) $MAX_VQ sets the maximum vq and smaller vqs are
all supported when $VLS_MAP is zero, or when the vqs are selected
in $VLS_MAP.

(2) is the same as (1) except KVM has the final say on what
vqs are valid.

(3) doesn't accept sve-max-vq because a guest that uses this
property without sve-vls-map cannot be safely migrated.

There is never any need to provide both properties, but if both
are provided then they are checked for consistency.

The QMP query returns a list of valid vq lists. For example, if
a guest can use vqs 1, 2, 3, and 4, then the following list will
be returned

 [ [ 1 ], [ 1, 2 ], [ 1, 2, 3 ], [ 1, 2, 3, 4 ] ]

Another example might be 1, 2, 4, as the architecture states 3
is optional. In that case the list would be

 [ [ 1 ], [ 1, 2 ], [ 1, 2, 4 ] ]

This may look redundant, but it's necessary to provide a future-
proof query, because while KVM currently requires vector sets to
be strict truncations of the full valid vector set, that may change
at some point. Additionally, TCG doesn't have this restriction so
more vector sets can be returned that aren't so easily derived from
the full set already. (Note, this series doesn't do that yet. See
the TODO below.)

And now for what might be a bit more controversial; how we input
the valid vector set with sve-vls-map. Well, sve-vls-map is a
64-bit bitmap, which is admittedly not user friendly and also not
the same size as KVM's vls bitmap (which is 8 64-bit words). Here's
the justification:

 1) We want to use the QEMU command line in order for the information
to be migrated without needing to add more VM state.
 2) It's not easy/pretty to input arrays on the QEMU command line.
 3) We already need to use the QMP query to get a valid set, which
isn't user friendly either, meaning we're already in libvirt
territory.
 4) A 64-bit map (supporting up to 8192-bit vectors) is likely big
enough for quite some time (currently KVM and TCG only support
2048-bit vectors).
 5) If user friendliness is needed more than migratability then
the 'max' cpu type can be used with the sve-max-vq property.
 6) It's possible to probe the full valid vector set from the
command line by using something like sve-vls-map=0x and
then, when it fails, the error message will state the correct
map, e.g. 0xb.

TODO:
 1) More testing. Initial testing looks good, and I'm doing more
now, but wanted to get the series out for review in parallel.
 2) Extension to target/arm/arch_dump.c for SVE state. I'll try
to get to this early next week.
 3) Modify the QMP query to output all valid vector sets for
TCG, rather than just the ones derived by truncation as
is required by KVM.

[1] https://www.spinics.net/lists/kvm-arm/msg35844.html

Thanks!

Andrew Jones (13):
  target/arm/kvm64: fix error returns
  update-linux-headers: Add sve_context.h to asm-arm64
  HACK: linux header update
  target/arm/kvm: Move the get/put of fpsimd registers out
  target/arm/kvm: Add kvm_arch_get/put_sve
  target/arm/kvm: max cpu: Enable SVE when available
  target/arm/kvm: max cpu: Allow sve max vector length setting
  target/arm/monitor: Add query-sve-vector-lengths
  target/arm/kvm: Export kvm_arm_get_sve_vls
  target/arm/monitor: kvm: only return valid sve vector sets
  target/arm/cpu64: max cpu: Introduce sve-vls-map
  target/arm/kvm: max cpu: Add support for sve-vls-map
  target/arm/kvm: host cpu: Add support for sve-vls-map

 linux-headers/asm-arm64/kvm.h |  41 +++
 linux-headers/asm-arm64/sve_context.h |  53 
 linux-headers/linux/kvm.h |   5 +
 qapi/target.json  |  34 +++
 scripts/update-linux-headers.sh   |   3 +
 target/arm/cpu.c  |   5 +
 target/arm/cpu.h  |   9 +
 target/arm/cpu64.c|  81 -
 target/arm/helper.c   |  10 +-
 target/arm/kvm.c  |   5 +
 target/arm/kvm64.c| 418 ++
 target/arm/kvm_arm.h  |  32 ++
 target/arm/monitor.c  

Re: [Qemu-devel] [PATCH 0/2]: vmdk: Add read-only support for the new seSparse format

2019-05-12 Thread Sam
Gentle ping on "[PATCH 2/2] vmdk: Add read-only support for seSparse snapshots”.
Yuchenlin reviewed "[PATCH 1/2] vmdk: Fix comment regarding max l1_size 
coverage”.

Thanks, Sam

> On 24 Apr 2019, at 10:48, Sam Eiderman  wrote:
> 
> VMware introduced a new snapshot format in VMFS6 - seSparse (Space
> Efficient Sparse) which is the default format available in ESXi 6.7.
> Add read-only support for the new snapshot format.
>