Re: [Qemu-devel] [qemu-s390x] [PATCH v3 06/10] hw/s390x: Replace global smp variables with machine smp properties
On 06.06.19 04:56, Eduardo Habkost wrote:
> On Sun, May 19, 2019 at 04:54:24AM +0800, Like Xu wrote:
>> The global smp variables in s390x are replaced with smp machine properties.
>>
>> A local variable of the same name would be introduced in the declaration
>> phase if it's used widely in the context OR replace it on the spot if it's
>> only used once. No semantic changes.
>>
>> Signed-off-by: Like Xu
>
> CCing s390 maintainers.
>
> Any objections to merging this through the Machine Core tree?
Fine with me.
Acked-by: Christian Borntraeger
>
>> ---
>> hw/s390x/s390-virtio-ccw.c | 3 ++-
>> hw/s390x/sclp.c| 2 +-
>> target/s390x/cpu.c | 3 +++
>> target/s390x/excp_helper.c | 5 +
>> 4 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index ed1fe7a93e..692ad6e372 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -83,7 +83,7 @@ static void s390_init_cpus(MachineState *machine)
>> /* initialize possible_cpus */
>> mc->possible_cpu_arch_ids(machine);
>>
>> -for (i = 0; i < smp_cpus; i++) {
>> +for (i = 0; i < machine->smp.cpus; i++) {
>> s390x_new_cpu(machine->cpu_type, i, &error_fatal);
>> }
>> }
>> @@ -410,6 +410,7 @@ static CpuInstanceProperties
>> s390_cpu_index_to_props(MachineState *ms,
>> static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms)
>> {
>> int i;
>> +unsigned int max_cpus = ms->smp.max_cpus;
>>
>> if (ms->possible_cpus) {
>> g_assert(ms->possible_cpus && ms->possible_cpus->len == max_cpus);
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 4510a800cb..fac7c3bb6c 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -64,7 +64,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>> prepare_cpu_entries(sclp, read_info->entries, &cpu_count);
>> read_info->entries_cpu = cpu_to_be16(cpu_count);
>> read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
>> -read_info->highest_cpu = cpu_to_be16(max_cpus - 1);
>> +read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
>>
>> read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index b1df63d82c..f1e5c0d9c3 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -37,6 +37,7 @@
>> #include "hw/qdev-properties.h"
>> #ifndef CONFIG_USER_ONLY
>> #include "hw/hw.h"
>> +#include "hw/boards.h"
>> #include "sysemu/arch_init.h"
>> #include "sysemu/sysemu.h"
>> #endif
>> @@ -193,6 +194,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error
>> **errp)
>> }
>>
>> #if !defined(CONFIG_USER_ONLY)
>> +MachineState *ms = MACHINE(qdev_get_machine());
>> +unsigned int max_cpus = ms->smp.max_cpus;
>> if (cpu->env.core_id >= max_cpus) {
>> error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
>> ", maximum core-id: %d", cpu->env.core_id,
>> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
>> index 3a467b72c5..1c6938effc 100644
>> --- a/target/s390x/excp_helper.c
>> +++ b/target/s390x/excp_helper.c
>> @@ -31,6 +31,7 @@
>> #ifndef CONFIG_USER_ONLY
>> #include "sysemu/sysemu.h"
>> #include "hw/s390x/s390_flic.h"
>> +#include "hw/boards.h"
>> #endif
>>
>> void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t
>> code,
>> @@ -300,6 +301,10 @@ static void do_ext_interrupt(CPUS390XState *env)
>> g_assert(cpu_addr < S390_MAX_CPUS);
>> lowcore->cpu_addr = cpu_to_be16(cpu_addr);
>> clear_bit(cpu_addr, env->emergency_signals);
>> +#ifndef CONFIG_USER_ONLY
>> +MachineState *ms = MACHINE(qdev_get_machine());
>> +unsigned int max_cpus = ms->smp.max_cpus;
>> +#endif
>> if (bitmap_empty(env->emergency_signals, max_cpus)) {
>> env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
>> }
>> --
>> 2.21.0
>>
>>
>
Re: [Qemu-devel] [PATCH v2 5/5] block/nvme: add support for discard
On Thu, 2019-06-06 at 11:19 +0800, Fam Zheng wrote:
> On Wed, 04/17 22:53, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky
> > ---
> > block/nvme.c | 80 ++
> > block/trace-events | 2 ++
> > 2 files changed, 82 insertions(+)
> >
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 35b925899f..b83912c627 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> > @@ -110,6 +110,7 @@ typedef struct {
> > bool plugged;
> >
> > bool supports_write_zeros;
> > +bool supports_discard;
> >
> > CoMutex dma_map_lock;
> > CoQueue dma_flush_queue;
> > @@ -462,6 +463,7 @@ static void nvme_identify(BlockDriverState *bs, int
> > namespace, Error **errp)
> >
> >
> > s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
> > +s->supports_discard = (idctrl->oncs & NVME_ONCS_DSM) != 0;
> >
> > memset(resp, 0, 4096);
> >
> > @@ -1144,6 +1146,83 @@ static coroutine_fn int
> > nvme_co_pwrite_zeroes(BlockDriverState *bs,
> > }
> >
> >
> > +static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
> > +int64_t offset, int bytes)
>
> While you respin, you can align the parameters.
Hi Fam!!
I didn't knew that this is also required by qemu codeing style (it kind of
suggested in the kernel)
I'll be more that glad to do so!
>
> > +{
> > +BDRVNVMeState *s = bs->opaque;
> > +NVMeQueuePair *ioq = s->queues[1];
> > +NVMeRequest *req;
> > +NvmeDsmRange *buf;
> > +QEMUIOVector local_qiov;
> > +int r;
> > +
> > +NvmeCmd cmd = {
> > +.opcode = NVME_CMD_DSM,
> > +.nsid = cpu_to_le32(s->nsid),
> > +.cdw10 = 0, /*number of ranges - 0 based*/
> > +.cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
> > +};
> > +
> > +NVMeCoData data = {
> > +.ctx = bdrv_get_aio_context(bs),
> > +.ret = -EINPROGRESS,
> > +};
> > +
> > +if (!s->supports_discard) {
> > +return -ENOTSUP;
> > +}
> > +
> > +assert(s->nr_queues > 1);
> > +
> > +buf = qemu_try_blockalign0(bs, 4096);
> > +if (!buf) {
> > +return -ENOMEM;
> > +}
> > +
> > +buf->nlb = bytes >> s->blkshift;
> > +buf->slba = offset >> s->blkshift;
>
> This buffer is for the device, do we need to do anything about the endianness?
Thank you very very much, this is indeed an endianess bug.
Thanks a lot for the review,
Best regards,
Maxim Levitsky
>
> > +buf->cattr = 0;
> > +
> > +qemu_iovec_init(&local_qiov, 1);
> > +qemu_iovec_add(&local_qiov, buf, 4096);
> > +
> > +req = nvme_get_free_req(ioq);
> > +assert(req);
> > +
> > +qemu_co_mutex_lock(&s->dma_map_lock);
> > +r = nvme_cmd_map_qiov(bs, &cmd, req, &local_qiov);
> > +qemu_co_mutex_unlock(&s->dma_map_lock);
> > +
> > +if (r) {
> > +req->busy = false;
> > +return r;
> > +}
> > +
> > +trace_nvme_dsm(s, offset, bytes);
> > +
> > +nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
> > +
> > +data.co = qemu_coroutine_self();
> > +while (data.ret == -EINPROGRESS) {
> > +qemu_coroutine_yield();
> > +}
> > +
> > +qemu_co_mutex_lock(&s->dma_map_lock);
> > +r = nvme_cmd_unmap_qiov(bs, &local_qiov);
> > +qemu_co_mutex_unlock(&s->dma_map_lock);
> > +if (r) {
> > +return r;
> > +}
> > +
> > +trace_nvme_dsm_done(s, offset, bytes, data.ret);
> > +
> > +qemu_iovec_destroy(&local_qiov);
> > +qemu_vfree(buf);
> > +return data.ret;
> > +
> > +}
> > +
> > +
> > static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
> > BlockReopenQueue *queue, Error **errp)
> > {
> > @@ -1250,6 +1329,7 @@ static BlockDriver bdrv_nvme = {
> > .bdrv_co_pwritev = nvme_co_pwritev,
> >
> > .bdrv_co_pwrite_zeroes= nvme_co_pwrite_zeroes,
> > +.bdrv_co_pdiscard = nvme_co_pdiscard,
> >
> > .bdrv_co_flush_to_disk= nvme_co_flush,
> > .bdrv_reopen_prepare = nvme_reopen_prepare,
> > diff --git a/block/trace-events b/block/trace-events
> > index 943a58569f..e55ac5c40b 100644
> > --- a/block/trace-events
> > +++ b/block/trace-events
> > @@ -148,6 +148,8 @@ nvme_write_zeros(void *s, uint64_t offset, uint64_t
> > bytes, int flags) "s %p offs
> > nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int
> > align) "qiov %p n %d base %p size 0x%zx align 0x%x"
> > nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int
> > is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
> > nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int
> > ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
> > +nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset %"PRId64"
> > bytes %"PRId64""
> > +nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p
> > offset %"PRId6
Re: [Qemu-devel] [PATCH v4 08/11] numa: Extend the command-line to provide memory latency and bandwidth information
On 6/5/2019 10:40 PM, Igor Mammedov wrote:
On Wed, 8 May 2019 14:17:23 +0800
Tao Xu wrote:
From: Liu Jingqi
Add -numa hmat-lb option to provide System Locality Latency and
Bandwidth Information. These memory attributes help to build
System Locality Latency and Bandwidth Information Structure(s)
in ACPI Heterogeneous Memory Attribute Table (HMAT).
Signed-off-by: Liu Jingqi
Signed-off-by: Tao Xu
---
...
+##
+{ 'struct': 'NumaHmatLBOptions',
+ 'data': {
+ 'initiator': 'uint16',
+ 'target': 'uint16',
+ 'hierarchy': 'HmatLBMemoryHierarchy',
+ 'data-type': 'HmatLBDataType',
I think union will be better here with data-type used as discriminator,
on top of that you'll be able to drop a bit of error checking above since
QAPI's union will not allow user to mix latency and bandwidth.
Hi Igor,
I have quesion here, the 'hmat-lb' is a member of a union 'NumaOptions',
it seems can' use a union as a member of union.
Re: [Qemu-devel] [Qemu-ppc] [PATCH v9 5/6] ppc: spapr: Enable FWNMI capability
On Thursday 06 June 2019 08:32 AM, David Gibson wrote:
> On Tue, Jun 04, 2019 at 12:15:26PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Monday 03 June 2019 08:55 PM, Greg Kurz wrote:
>>> On Wed, 29 May 2019 11:10:49 +0530
>>> Aravinda Prasad wrote:
>>>
Enable the KVM capability KVM_CAP_PPC_FWNMI so that
the KVM causes guest exit with NMI as exit reason
when it encounters a machine check exception on the
address belonging to a guest. Without this capability
enabled, KVM redirects machine check exceptions to
guest's 0x200 vector.
This patch also deals with the case when a guest with
the KVM_CAP_PPC_FWNMI capability enabled is attempted
to migrate to a host that does not support this
capability.
Signed-off-by: Aravinda Prasad
---
>>>
>>> As suggested in another mail, it may be worth introducing the sPAPR cap
>>> in its own patch, earlier in the series.
>>
>> Sure, also as a workaround mentioned in the reply to that mail, I am
>> thinking of returning RTAS_OUT_NOT_SUPPORTED to rtas nmi register call
>> until the entire functionality is implemented. This will help solve
>> spapr cap issue as well.
>
> Not registering the RTAS call at all is the correct way to handle that
> case.
>
>>
>>>
>>> Anyway, I have some comments below.
>>>
hw/ppc/spapr.c |1 +
hw/ppc/spapr_caps.c| 24
hw/ppc/spapr_rtas.c| 18 ++
include/hw/ppc/spapr.h |4 +++-
target/ppc/kvm.c | 19 +++
target/ppc/kvm_ppc.h | 12
6 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c97f6a6..e8a77636 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4364,6 +4364,7 @@ static void spapr_machine_class_init(ObjectClass
*oc, void *data)
smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF;
+smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
spapr_caps_add_properties(smc, &error_abort);
smc->irq = &spapr_irq_dual;
smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 31b4661..ef9e612 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -479,6 +479,20 @@ static void cap_ccf_assist_apply(SpaprMachineState
*spapr, uint8_t val,
}
}
+static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
+Error **errp)
+{
+if (!val) {
+return; /* Disabled by default */
+}
+
+if (tcg_enabled()) {
+error_setg(errp, "No fwnmi support in TCG, try
cap-fwnmi-mce=off");
>>>
>>> Maybe expand "fwnmi" to "Firmware Assisted Non-Maskable Interrupts" ?
>>
>> sure..
>>
>>>
+} else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
+error_setg(errp, "Requested fwnmi capability not support by
KVM");
>>>
>>> Maybe reword and add a hint:
>>>
>>> "KVM implementation does not support Firmware Assisted Non-Maskable
>>> Interrupts, try cap-fwnmi-mce=off"
>>
>> sure..
>>
>>>
>>>
+}
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -578,6 +592,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] =
{
.type = "bool",
.apply = cap_ccf_assist_apply,
},
+[SPAPR_CAP_FWNMI_MCE] = {
+.name = "fwnmi-mce",
+.description = "Handle fwnmi machine check exceptions",
+.index = SPAPR_CAP_FWNMI_MCE,
+.get = spapr_cap_get_bool,
+.set = spapr_cap_set_bool,
+.type = "bool",
+.apply = cap_fwnmi_mce_apply,
+},
};
static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -717,6 +740,7 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize,
SPAPR_CAP_HPT_MAXPAGESIZE);
SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
+SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
void spapr_caps_init(SpaprMachineState *spapr)
{
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index e0bdfc8..91a7ab9 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -49,6 +49,7 @@
#include "hw/ppc/fdt.h"
#include "target/ppc/mmu-hash64.h"
#include "target/ppc/mmu-book3s-v3.h"
+#include "kvm_ppc.h"
static void rtas_display_character(PowerPCCPU *cpu, SpaprMa
Re: [Qemu-devel] [PATCH v3 06/10] hw/s390x: Replace global smp variables with machine smp properties
On Wed, 5 Jun 2019 23:56:35 -0300 Eduardo Habkost wrote: > On Sun, May 19, 2019 at 04:54:24AM +0800, Like Xu wrote: > > The global smp variables in s390x are replaced with smp machine properties. > > > > A local variable of the same name would be introduced in the declaration > > phase if it's used widely in the context OR replace it on the spot if it's > > only used once. No semantic changes. > > > > Signed-off-by: Like Xu > > CCing s390 maintainers. > > Any objections to merging this through the Machine Core tree? None at all. > > > --- > > hw/s390x/s390-virtio-ccw.c | 3 ++- > > hw/s390x/sclp.c| 2 +- > > target/s390x/cpu.c | 3 +++ > > target/s390x/excp_helper.c | 5 + > > 4 files changed, 11 insertions(+), 2 deletions(-) Reviewed-by: Cornelia Huck
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 04/10] hw/ppc: Replace global smp variables with machine smp properties
On Thu, 6 Jun 2019 13:07:32 +1000
David Gibson wrote:
> On Wed, Jun 05, 2019 at 11:54:56PM -0300, Eduardo Habkost wrote:
> > On Wed, Jun 05, 2019 at 11:52:41PM -0300, Eduardo Habkost wrote:
> > > On Sun, May 19, 2019 at 04:54:22AM +0800, Like Xu wrote:
> > > > The global smp variables in ppc are replaced with smp machine
> > > > properties.
> > > >
> > > > A local variable of the same name would be introduced in the declaration
> > > > phase if it's used widely in the context OR replace it on the spot if
> > > > it's
> > > > only used once. No semantic changes.
> > > >
> > > > Signed-off-by: Like Xu
> > >
> > > Any objections from the ppc maintainers to queueing this through
> > > the Machine Core tree?
> >
> > Oops, CCing the ppc maintainers.
>
> No objection here.
>
> Acked-by: David Gibson
>
Just one nit...
[...]
> > > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > > > index ee24212765..c9ffe9786c 100644
> > > > --- a/hw/ppc/spapr_rtas.c
> > > > +++ b/hw/ppc/spapr_rtas.c
> > > > @@ -231,6 +231,8 @@ static void
> > > > rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> > > >target_ulong args,
> > > >uint32_t nret, target_ulong
> > > > rets)
> > > > {
> > > > +MachineState *ms = MACHINE(qdev_get_machine());
rtas_ibm_get_system_parameter() has a SpaprMachineState *spapr argument, no
need to rely on qdev_get_machine().
But this can be fixed in a followup patch I guess. Not worth holding the
patchset because of that.
> > > > +unsigned int max_cpus = ms->smp.max_cpus;
> > > > target_ulong parameter = rtas_ld(args, 0);
> > > > target_ulong buffer = rtas_ld(args, 1);
> > > > target_ulong length = rtas_ld(args, 2);
> > > > @@ -244,7 +246,7 @@ static void
> > > > rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> > > >"MaxPlatProcs=%d",
> > > >max_cpus,
> > > >current_machine->ram_size /
> > > > MiB,
> > > > - smp_cpus,
> > > > + ms->smp.cpus,
> > > >max_cpus);
> > > > ret = sysparm_st(buffer, length, param_val, strlen(param_val)
> > > > + 1);
> > > > g_free(param_val);
> > >
> >
>
pgpcouHFNy6Zd.pgp
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 2/6] migration/multifd: notify channels_ready when send thread starts
multifd_send_state->channels_ready is initialized to 0. It is proper to
let main thread know we are ready when thread start running.
Current implementation works since ram_save_setup() calls
multifd_send_sync_main() which wake up send thread and posts
channels_ready. This behavior will introduce some unpredictable
situation and disturb the semaphore value.
This is a preparation patch to use another mechanism to do send thread
synchronization to avoid post channels_ready in this case. So this patch
posts channels_ready when send threads start running.
Signed-off-by: Wei Yang
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index a4e7587648..f9e53ac413 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1093,6 +1093,8 @@ static void *multifd_send_thread(void *opaque)
}
/* initial packet */
p->num_packets = 1;
+/* let main thread know we are ready */
+qemu_sem_post(&multifd_send_state->channels_ready);
while (true) {
qemu_sem_wait(&p->sem);
--
2.19.1
Re: [Qemu-devel] [PATCH 07/13] target/arm/kvm: max cpu: Allow sve max vector length setting
Hi Drew,
On 5/12/19 10:36 AM, Andrew Jones wrote:
> 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, &error_fatal);
> }
> +
> +object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
> +cpu_max_set_sve_vq, NULL, NULL, &error_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)&sve_vls[0],
> +};
> +int i, ret;
> +
> +ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> +if (ret) {
> +return ret;
> +}
> +
> +ret = 0;
is it mandated?
> +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)&sve_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;
nit: add a line
> +while (vq > max_vq) {
> +sve_vls[i] &= ~BIT_MASK(vq - 1);
Isn't BIT_MASK for 32b. MAKE_64BIT_MASK?
> +vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
> +}
> +if (vq < max_vq) {
I don't really get this check: having vq less than max_vq does not seems
weird. Do you absolutely want vq=max_vq?
> +error_report("sve-max-vq=%d is not a valid length", max_vq);
> +error_printf("next lowest is %d\n", vq);
why mixing error_report/printf?
> +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)
> }
[Qemu-devel] [PATCH 4/6] migration/multifd: used must not be 0 for a pending job
After thread synchronization request is handled in another case, this
means when we only get pending_job when there is used pages.
Signed-off-by: Wei Yang
---
migration/ram.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 9982930392..3e48795608 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1118,12 +1118,11 @@ static void *multifd_send_thread(void *opaque)
break;
}
-if (used) {
-ret = qio_channel_writev_all(p->c, p->pages->iov,
+assert(used);
+ret = qio_channel_writev_all(p->c, p->pages->iov,
used, &local_err);
-if (ret != 0) {
-break;
-}
+if (ret != 0) {
+break;
}
qemu_mutex_lock(&p->mutex);
--
2.19.1
[Qemu-devel] [PATCH 6/6] migration/multifd: there is no spurious wakeup now
The spurious wakeup is gone.
Signed-off-by: Wei Yang
---
migration/ram.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 831b15833b..2490631d52 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1153,9 +1153,6 @@ static void *multifd_send_thread(void *opaque)
} else if (p->quit) {
qemu_mutex_unlock(&p->mutex);
break;
-} else {
-qemu_mutex_unlock(&p->mutex);
-/* sometimes there are spurious wakeups */
}
}
--
2.19.1
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 04/10] hw/ppc: Replace global smp variables with machine smp properties
On 2019/6/6 16:20, Greg Kurz wrote:
On Thu, 6 Jun 2019 13:07:32 +1000
David Gibson wrote:
On Wed, Jun 05, 2019 at 11:54:56PM -0300, Eduardo Habkost wrote:
On Wed, Jun 05, 2019 at 11:52:41PM -0300, Eduardo Habkost wrote:
On Sun, May 19, 2019 at 04:54:22AM +0800, Like Xu wrote:
The global smp variables in ppc are replaced with smp machine properties.
A local variable of the same name would be introduced in the declaration
phase if it's used widely in the context OR replace it on the spot if it's
only used once. No semantic changes.
Signed-off-by: Like Xu
Any objections from the ppc maintainers to queueing this through
the Machine Core tree?
Oops, CCing the ppc maintainers.
No objection here.
Acked-by: David Gibson
Just one nit...
[...]
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ee24212765..c9ffe9786c 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -231,6 +231,8 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
target_ulong args,
uint32_t nret, target_ulong rets)
{
+MachineState *ms = MACHINE(qdev_get_machine());
rtas_ibm_get_system_parameter() has a SpaprMachineState *spapr argument, no
need to rely on qdev_get_machine().
I will fix it in the next (rebased) version. Thank you, Greg.
But this can be fixed in a followup patch I guess. Not worth holding the
patchset because of that.
+unsigned int max_cpus = ms->smp.max_cpus;
target_ulong parameter = rtas_ld(args, 0);
target_ulong buffer = rtas_ld(args, 1);
target_ulong length = rtas_ld(args, 2);
@@ -244,7 +246,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
"MaxPlatProcs=%d",
max_cpus,
current_machine->ram_size / MiB,
- smp_cpus,
+ ms->smp.cpus,
max_cpus);
ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
g_free(param_val);
[Qemu-devel] [PATCH 3/6] migration/multifd: use sync field to synchronize send threads
Add a field in MultiFDSendParams to indicate there is a request to
synchronize send threads.
By doing so, send_thread will just post sem_sync on synchronization
request and channels_ready will not *overflow*.
Signed-off-by: Wei Yang
---
migration/ram.c | 30 --
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index f9e53ac413..9982930392 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -640,6 +640,8 @@ typedef struct {
QemuMutex mutex;
/* is this channel thread running */
bool running;
+/* should sync this channel */
+bool sync;
/* should this thread finish */
bool quit;
/* thread has work to do */
@@ -1065,8 +1067,7 @@ static void multifd_send_sync_main(void)
qemu_mutex_lock(&p->mutex);
p->packet_num = multifd_send_state->packet_num++;
-p->flags |= MULTIFD_FLAG_SYNC;
-p->pending_job++;
+p->sync = true;
qemu_mutex_unlock(&p->mutex);
qemu_sem_post(&p->sem);
}
@@ -1129,10 +1130,27 @@ static void *multifd_send_thread(void *opaque)
p->pending_job--;
qemu_mutex_unlock(&p->mutex);
-if (flags & MULTIFD_FLAG_SYNC) {
-qemu_sem_post(&multifd_send_state->sem_sync);
-}
qemu_sem_post(&multifd_send_state->channels_ready);
+} else if (p->sync) {
+uint64_t packet_num = p->packet_num;
+uint32_t flags = p->flags;
+assert(!p->pages->used);
+
+p->flags |= MULTIFD_FLAG_SYNC;
+multifd_send_fill_packet(p);
+p->sync = false;
+qemu_mutex_unlock(&p->mutex);
+
+trace_multifd_send(p->id, packet_num, 0, flags | MULTIFD_FLAG_SYNC,
+ p->next_packet_size);
+
+ret = qio_channel_write_all(p->c, (void *)p->packet,
+p->packet_len, &local_err);
+if (ret != 0) {
+break;
+}
+
+qemu_sem_post(&multifd_send_state->sem_sync);
} else if (p->quit) {
qemu_mutex_unlock(&p->mutex);
break;
@@ -1196,7 +1214,7 @@ int multifd_save_setup(void)
qemu_mutex_init(&p->mutex);
qemu_sem_init(&p->sem, 0);
-p->quit = false;
+p->sync = p->quit = false;
p->pending_job = 0;
p->id = i;
p->pages = multifd_pages_init(page_count);
--
2.19.1
[Qemu-devel] [PATCH 0/6] multifd: a new mechanism for send thread sync
Current send thread could work while the sync mechanism has some problem: * has spuriously wakeup * number of channels_ready will *overflow* the number of real channels The reason is: * if MULTIFD_FLAG_SYNC is set in the middle of send thread running, there is one more spurious wakeup * if MULTIFD_FLAG_SYNC is set when send thread is not running, there is one more channels_ready be triggered To solve this situation, one new mechanism is introduced to synchronize send threads. The idea is simple, a new field *sync* is introduced to indicate a synchronization is required. Wei Yang (6): migration/multifd: move MultiFDSendParams handling into multifd_send_fill_packet() migration/multifd: notify channels_ready when send thread starts migration/multifd: use sync field to synchronize send threads migration/multifd: used must not be 0 for a pending job migration/multifd: use boolean for pending_job is enough migration/multifd: there is no spurious wakeup now migration/ram.c | 62 +++-- 1 file changed, 39 insertions(+), 23 deletions(-) -- 2.19.1
[Qemu-devel] [PATCH 5/6] migration/multifd: use boolean for pending_job is enough
After synchronization request is handled in another case, there only
could be one pending_job for one send thread at most.
This is fine to use boolean to represent this behavior.
Signed-off-by: Wei Yang
---
migration/ram.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 3e48795608..831b15833b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -645,7 +645,7 @@ typedef struct {
/* should this thread finish */
bool quit;
/* thread has work to do */
-int pending_job;
+bool pending_job;
/* array of pages to sent */
MultiFDPages_t *pages;
/* packet allocated len */
@@ -942,7 +942,7 @@ static void multifd_send_pages(void)
qemu_mutex_lock(&p->mutex);
if (!p->pending_job) {
-p->pending_job++;
+p->pending_job = true;
next_channel = (i + 1) % migrate_multifd_channels();
break;
}
@@ -1126,7 +1126,7 @@ static void *multifd_send_thread(void *opaque)
}
qemu_mutex_lock(&p->mutex);
-p->pending_job--;
+p->pending_job = false;
qemu_mutex_unlock(&p->mutex);
qemu_sem_post(&multifd_send_state->channels_ready);
@@ -1213,8 +1213,7 @@ int multifd_save_setup(void)
qemu_mutex_init(&p->mutex);
qemu_sem_init(&p->sem, 0);
-p->sync = p->quit = false;
-p->pending_job = 0;
+p->sync = p->quit = p->pending_job = false;
p->id = i;
p->pages = multifd_pages_init(page_count);
p->packet_len = sizeof(MultiFDPacket_t)
--
2.19.1
Re: [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems
Aleksandar Markovic writes: > On Jun 5, 2019 11:03 PM, "Richard Henderson" > wrote: >> >> This is a collection of related > > Related to what? > >> defines for notes, copied >> from glibc's . We're not going to use all of these >> right away, but it seemed foolish > > I don't think this an appropriate word for a commit message. Calling an alternative you considered but rejected "foolish" feels perfectly alright to me. >> to cherry-pick only the >> ones we need now. >> >> Signed-off-by: Richard Henderson
[Qemu-devel] [PATCH] MAINTAINERS: put myself forward for gdbstub
As I've been reviewing a lot of this recently and I'm going to put together a pull request I'd better keep an eye on it. Signed-off-by: Alex Bennée --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index a96829ea83..8ef34cf1ce 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1872,7 +1872,8 @@ F: util/error.c F: util/qemu-error.c GDB stub -S: Orphan +M: Alex Bennée +S: Maintained F: gdbstub* F: gdb-xml/ -- 2.20.1
[Qemu-devel] [PATCH 1/6] migration/multifd: move MultiFDSendParams handling into multifd_send_fill_packet()
Currently there is only one user of multifd_send_fill_packet(). We
enlarge the responsibility of it to adjust MultiFDSendParams.
Signed-off-by: Wei Yang
---
migration/ram.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index bd356764ff..a4e7587648 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -787,9 +787,11 @@ static void multifd_pages_clear(MultiFDPages_t *pages)
static void multifd_send_fill_packet(MultiFDSendParams *p)
{
MultiFDPacket_t *packet = p->packet;
+MultiFDPages_t *pages = p->pages;
uint32_t page_max = MULTIFD_PACKET_SIZE / qemu_target_page_size();
int i;
+p->next_packet_size = pages->used * qemu_target_page_size();
packet->magic = cpu_to_be32(MULTIFD_MAGIC);
packet->version = cpu_to_be32(MULTIFD_VERSION);
packet->flags = cpu_to_be32(p->flags);
@@ -805,6 +807,10 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
for (i = 0; i < p->pages->used; i++) {
packet->offset[i] = cpu_to_be64(p->pages->offset[i]);
}
+p->flags = 0;
+p->num_packets++;
+p->num_pages += pages->used;
+p->pages->used = 0;
}
static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
@@ -1097,12 +1103,7 @@ static void *multifd_send_thread(void *opaque)
uint64_t packet_num = p->packet_num;
uint32_t flags = p->flags;
-p->next_packet_size = used * qemu_target_page_size();
multifd_send_fill_packet(p);
-p->flags = 0;
-p->num_packets++;
-p->num_pages += used;
-p->pages->used = 0;
qemu_mutex_unlock(&p->mutex);
trace_multifd_send(p->id, packet_num, used, flags,
--
2.19.1
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
* Liran Alon (liran.a...@oracle.com) wrote: > Hi, > > Looking at QEMU source code, I am puzzled regarding how migration backwards > compatibility is preserved regarding X86CPU. > > As I understand it, fields that are based on KVM capabilities and guest > runtime usage are defined in VMState subsections in order to not send them if > not necessary. > This is done such that in case they are not needed and we migrate to an old > QEMU which don’t support loading this state, migration will still succeed > (As .needed() method will return false and therefore this state won’t be sent > as part of migration stream). > Furthermore, in case .needed() returns true and old QEMU don’t support > loading this state, migration fails. As it should because we are aware that > guest state > is not going to be restored properly on destination. > > I’m puzzled about what will happen in the following scenario: > 1) Source is running new QEMU with new KVM that supports save of some VMState > subsection. > 2) Destination is running new QEMU that supports load this state but with old > kernel that doesn’t know how to load this state. > > I would have expected in this case that if source .needed() returns true, > then migration will fail because of lack of support in destination kernel. > However, it seems from current QEMU code that this will actually succeed in > many cases. > > For example, if msr_smi_count is sent as part of migration stream (See > vmstate_msr_smi_count) and destination have has_msr_smi_count==false, > then destination will succeed loading migration stream but kvm_put_msrs() > will actually ignore env->msr_smi_count and will successfully load guest > state. > Therefore, migration will succeed even though it should have failed… > > It seems to me that QEMU should have for every such VMState subsection, a > .post_load() method that verifies that relevant capability is supported by > kernel > and otherwise fail migration. > > What do you think? Should I really create a patch to modify all these CPUX86 > VMState subsections to behave like this? I don't know the x86 specific side that much; but from my migration side the answer should mostly be through machine types - indeed for smi-count there's a property 'x-migrate-smi-count' which is off for machine types pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old kernel you should stick to the old machine types. There's nothing guarding running the new machine type on old-kernels; and arguably we should have a check at startup that complains if your kernel is missing something the machine type uses. However, that would mean that people running with -M pc would fail on old kernels. A post-load is also a valid check; but one question is whether, for a particular register, the pain is worth it - it depends on the symptom that the missing state causes. If it's minor then you might conclude it's not worth a failed migration; if it's a hung or corrupt guest then yes it is. Certainly a warning printed is worth it. Dave > Thanks, > -Liran -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 07/13] target/arm/kvm: max cpu: Allow sve max vector length setting
On Thu, Jun 06, 2019 at 10:30:00AM +0200, Auger Eric wrote:
> Hi Drew,
> On 5/12/19 10:36 AM, Andrew Jones wrote:
> > 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, &error_fatal);
> > }
> > +
> > +object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_vq,
> > +cpu_max_set_sve_vq, NULL, NULL, &error_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)&sve_vls[0],
> > +};
> > +int i, ret;
> > +
> > +ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> > +if (ret) {
> > +return ret;
> > +}
> > +
> > +ret = 0;
> is it mandated?
Yes, this can be removed, as we know ret=0 here.
> > +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)&sve_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;
> nit: add a line
> > +while (vq > max_vq) {
> > +sve_vls[i] &= ~BIT_MASK(vq - 1);
> Isn't BIT_MASK for 32b. MAKE_64BIT_MASK?
We should just need a 'UL', not a 'ULL'. I think I'll change all
my BIT_MASK() usage to just BIT() for v2 though, even though
there doesn't appear to be a difference for the cases I've used
it.
> > +vq = arm_cpu_fls64(sve_vls[i]) + i * 64;
> > +}
> > +if (vq < max_vq) {
> I don't really get this check: having vq less than max_vq does not seems
> weird. Do you absolutely want vq=max_vq?
In this context 'vq' is the max vq KVM supports and max_vq is the maximum
the user requested. So if the user wants a maximum greater than what is
possible, then that's an error.
> > +error_report("sve-max-vq=%d is not a valid length",
> > max_vq);
> > +error_printf("next lowest is %d\n", vq);
> why mixing error_report/printf?
This is how we're supposed to do two line error messages, afaik.
> > +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(OBJEC
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
> On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> Hi, >> >> Looking at QEMU source code, I am puzzled regarding how migration backwards >> compatibility is preserved regarding X86CPU. >> >> As I understand it, fields that are based on KVM capabilities and guest >> runtime usage are defined in VMState subsections in order to not send them >> if not necessary. >> This is done such that in case they are not needed and we migrate to an old >> QEMU which don’t support loading this state, migration will still succeed >> (As .needed() method will return false and therefore this state won’t be >> sent as part of migration stream). >> Furthermore, in case .needed() returns true and old QEMU don’t support >> loading this state, migration fails. As it should because we are aware that >> guest state >> is not going to be restored properly on destination. >> >> I’m puzzled about what will happen in the following scenario: >> 1) Source is running new QEMU with new KVM that supports save of some >> VMState subsection. >> 2) Destination is running new QEMU that supports load this state but with >> old kernel that doesn’t know how to load this state. >> >> I would have expected in this case that if source .needed() returns true, >> then migration will fail because of lack of support in destination kernel. >> However, it seems from current QEMU code that this will actually succeed in >> many cases. >> >> For example, if msr_smi_count is sent as part of migration stream (See >> vmstate_msr_smi_count) and destination have has_msr_smi_count==false, >> then destination will succeed loading migration stream but kvm_put_msrs() >> will actually ignore env->msr_smi_count and will successfully load guest >> state. >> Therefore, migration will succeed even though it should have failed… >> >> It seems to me that QEMU should have for every such VMState subsection, a >> .post_load() method that verifies that relevant capability is supported by >> kernel >> and otherwise fail migration. >> >> What do you think? Should I really create a patch to modify all these CPUX86 >> VMState subsections to behave like this? > > I don't know the x86 specific side that much; but from my migration side > the answer should mostly be through machine types - indeed for smi-count > there's a property 'x-migrate-smi-count' which is off for machine types > pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old > kernel you should stick to the old machine types. > > There's nothing guarding running the new machine type on old-kernels; > and arguably we should have a check at startup that complains if > your kernel is missing something the machine type uses. > However, that would mean that people running with -M pc would fail > on old kernels. > > A post-load is also a valid check; but one question is whether, > for a particular register, the pain is worth it - it depends on the > symptom that the missing state causes. If it's minor then you might > conclude it's not worth a failed migration; if it's a hung or > corrupt guest then yes it is. Certainly a warning printed is worth > it. > > Dave I think we should have flags that allow user to specify which VMState subsections user explicitly allow to avoid restore even though they are required to fully restore guest state. But it seems to me that the behaviour should be to always fail migration in case we load a VMState subsections that we are unable to restore unless user explicitly specified this is ok for this specific subsection. Therefore, it seems that for every VMState subsection that it’s restore is based on kernel capability we should: 1) Have a user-controllable flag (which is also tied to machine-type?) to explicitly allow avoid restoring this state if cannot. Default should be “false”. 2) Have a .post_load() method that verifies we have required kernel capability to restore this state, unless flag (1) was specified as “true”. Note that above mentioned flags is different than flags such as “x-migrate-smi-count”. The purpose of “x-migrate-smi-count” flag is to avoid sending the VMState subsection to begin with in case we know we migrate to older QEMU which don’t even have the relevant VMState subsection. But it is not relevant for the case both source and destination runs QEMU which understands the VMState subsection but run on kernels with different capabilities. Also note regarding your first paragraph, that specifying flags based on kernel you are running on doesn’t help for the case discussed here. As source QEMU is running on new kernel. Unless you meant that source QEMU should use relevant machine-type based on the destination kernel. i.e. You should launch QEMU with old machine-type as long as you have hosts in your migration pool that runs with old kernel. I don’ think it’s the right approach though. As there is no way to change flags such as “x-migrate-smi-count” dy
Re: [Qemu-devel] [PATCH v4 0/3] numa: deprecate '-numa node, mem' and default memory distribution
On Wed, Jun 05, 2019 at 03:06:08PM -0300, Eduardo Habkost wrote: > On Wed, Jun 05, 2019 at 06:33:11PM +0100, Daniel P. Berrangé wrote: > [...] > > I wonder if there's a way to close the testing gap somehow ? Random idea > > would be a non-versioned "pc-no-deprecated" machine type, which blocks > > all use of deprecated features and does not promise any migration compat. > > Essentially it would exist just for testing purposem as a way todo > > functional tests for libvirt & mgmt apps to prove they don't use any > > deprecated features (any deprecated features, not merely this NUMA one). > > This isn't the first time I wish we had a machine type with > experimental features enabled. What about calling it "pc-next"? No objection from me. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] Deprecation policy and build dependencies
On Wed, Jun 05, 2019 at 03:42:39PM -0500, Eric Blake wrote: > On 6/5/19 3:13 PM, Eduardo Habkost wrote: > > >> IOW, I don't think RHEL-7 support as a build platform blocks us from > >> dropping py2. We merely need to tweak our build platforms doc to clarify > >> our intent wrt add-on yum repos. > > > > If we clarify the docs in QEMU 4.1, is there anything that > > prevents us from removing Python 2 support in QEMU 4.1 too? > > My take (but not definitive): if we have any CI setups that are testing > RHEL 7 without software collections and/or EPEL, then save Python 2 > removal for 4.2 to give us time to update CI setups. But if all of our > CI setups are already fine, and we clarify the docs, then I'm all for > getting rid of Python 2 support in 4.1. The centos7 dockerfile will need to add the extra repo to pull this in. I don't see any issue with that getting fixed in this cycle. As for any CI maintained by third parties outside QEMU tree, they'll just have to adapt themselves. It is not difficult to add repos, so again I don't see a big reason to delay. If we delay chances are they won't bother to update their CI at all until we make the py3 change in 4.2 anyway. > Similarly, if we are going to outlaw in-tree builds, let's get that done > in 4.1 instead of waiting yet another release. I don't think there was any hard objection, someone just needs to write the patch... Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [Qemu-trivial] [PATCH] vhost: fix incorrect print type
Le 29/05/2019 à 15:13, Philippe Mathieu-Daudé a écrit :
> Cc'ing qemu-trivial
>
> On 4/30/19 10:48 AM, Philippe Mathieu-Daudé wrote:
>> On 4/30/19 8:29 AM, Jie Wang wrote:
>>> fix incorrect print type in vhost_virtqueue_stop
>>>
>>> Signed-off-by: Jie Wang
>>> ---
>>> hw/virtio/vhost.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>> index 7f61018f2a..286bb27c65 100644
>>> --- a/hw/virtio/vhost.c
>>> +++ b/hw/virtio/vhost.c
>>> @@ -1081,7 +1081,7 @@ static void vhost_virtqueue_stop(struct vhost_dev
>>> *dev,
>>>
>>> r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
>>> if (r < 0) {
>>> -VHOST_OPS_DEBUG("vhost VQ %d ring restore failed: %d", idx, r);
>>> +VHOST_OPS_DEBUG("vhost VQ %u ring restore failed: %d", idx, r);
>>
>> 'idx' is indeed unsigned.
>>
>> Reviewed-by: Philippe Mathieu-Daudé
>>
>>> /* Connection to the backend is broken, so let's sync internal
>>> * last avail idx to the device used idx.
>>> */
>>>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH] util: Adjust qemu_guest_getrandom_nofail for Coverity
Le 30/05/2019 à 19:38, Richard Henderson a écrit :
> Explicitly ignore the return value of qemu_guest_getrandom.
> Because we use error_fatal, all errors are already caught.
>
> Fixes: CID 1401701
> Signed-off-by: Richard Henderson
> ---
> util/guest-random.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/guest-random.c b/util/guest-random.c
> index e8124a3cad..00a08fd981 100644
> --- a/util/guest-random.c
> +++ b/util/guest-random.c
> @@ -56,7 +56,7 @@ int qemu_guest_getrandom(void *buf, size_t len, Error
> **errp)
>
> void qemu_guest_getrandom_nofail(void *buf, size_t len)
> {
> -qemu_guest_getrandom(buf, len, &error_fatal);
> +(void)qemu_guest_getrandom(buf, len, &error_fatal);
> }
>
> uint64_t qemu_guest_random_seed_thread_part1(void)
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH] test: Fix make target check-report.tap
Le 04/06/2019 à 10:42, Philippe Mathieu-Daudé a écrit : > On 6/4/19 10:28 AM, Paolo Bonzini wrote: >> On 04/06/19 10:09, Philippe Mathieu-Daudé wrote: check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check-report-unit.tap - $(call quiet-command,./scripts/tap-merge.py $^ > $@,"GEN","$@") + $(call quiet-command, cat $^ | scripts/tap-merge.pl >$@,"GEN","$@") >>> Paolo, did you intend to submit a new tap-merge.py script in commit >>> 9df43317b82? >>> >> >> No, it's really just y vs l. > > OK! > >> >> Reviewed-by: Paolo Bonzini >> Cc: qemu-triv...@nongnu.org > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > Applied to my trivial-patches branch. Thanks, Laurent
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
* Liran Alon (liran.a...@oracle.com) wrote: > > > > On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert wrote: > > > > * Liran Alon (liran.a...@oracle.com) wrote: > >> Hi, > >> > >> Looking at QEMU source code, I am puzzled regarding how migration > >> backwards compatibility is preserved regarding X86CPU. > >> > >> As I understand it, fields that are based on KVM capabilities and guest > >> runtime usage are defined in VMState subsections in order to not send them > >> if not necessary. > >> This is done such that in case they are not needed and we migrate to an > >> old QEMU which don’t support loading this state, migration will still > >> succeed > >> (As .needed() method will return false and therefore this state won’t be > >> sent as part of migration stream). > >> Furthermore, in case .needed() returns true and old QEMU don’t support > >> loading this state, migration fails. As it should because we are aware > >> that guest state > >> is not going to be restored properly on destination. > >> > >> I’m puzzled about what will happen in the following scenario: > >> 1) Source is running new QEMU with new KVM that supports save of some > >> VMState subsection. > >> 2) Destination is running new QEMU that supports load this state but with > >> old kernel that doesn’t know how to load this state. > >> > >> I would have expected in this case that if source .needed() returns true, > >> then migration will fail because of lack of support in destination kernel. > >> However, it seems from current QEMU code that this will actually succeed > >> in many cases. > >> > >> For example, if msr_smi_count is sent as part of migration stream (See > >> vmstate_msr_smi_count) and destination have has_msr_smi_count==false, > >> then destination will succeed loading migration stream but kvm_put_msrs() > >> will actually ignore env->msr_smi_count and will successfully load guest > >> state. > >> Therefore, migration will succeed even though it should have failed… > >> > >> It seems to me that QEMU should have for every such VMState subsection, a > >> .post_load() method that verifies that relevant capability is supported by > >> kernel > >> and otherwise fail migration. > >> > >> What do you think? Should I really create a patch to modify all these > >> CPUX86 VMState subsections to behave like this? > > > > I don't know the x86 specific side that much; but from my migration side > > the answer should mostly be through machine types - indeed for smi-count > > there's a property 'x-migrate-smi-count' which is off for machine types > > pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old > > kernel you should stick to the old machine types. > > > > There's nothing guarding running the new machine type on old-kernels; > > and arguably we should have a check at startup that complains if > > your kernel is missing something the machine type uses. > > However, that would mean that people running with -M pc would fail > > on old kernels. > > > > A post-load is also a valid check; but one question is whether, > > for a particular register, the pain is worth it - it depends on the > > symptom that the missing state causes. If it's minor then you might > > conclude it's not worth a failed migration; if it's a hung or > > corrupt guest then yes it is. Certainly a warning printed is worth > > it. > > > > Dave > > I think we should have flags that allow user to specify which VMState > subsections user explicitly allow to avoid restore even though they are > required to fully restore guest state. > But it seems to me that the behaviour should be to always fail migration in > case we load a VMState subsections that we are unable to restore unless user > explicitly specified this is ok > for this specific subsection. > Therefore, it seems that for every VMState subsection that it’s restore is > based on kernel capability we should: > 1) Have a user-controllable flag (which is also tied to machine-type?) to > explicitly allow avoid restoring this state if cannot. Default should be > “false”. > 2) Have a .post_load() method that verifies we have required kernel > capability to restore this state, unless flag (1) was specified as “true”. This seems a lot of flags; users aren't going to know what to do with all of them; I don't see what will set/control them. > Note that above mentioned flags is different than flags such as > “x-migrate-smi-count”. > The purpose of “x-migrate-smi-count” flag is to avoid sending the VMState > subsection to begin with in case we know we migrate to older QEMU which don’t > even have the relevant VMState subsection. But it is not relevant for the > case both source and destination runs QEMU which understands the VMState > subsection but run on kernels with different capabilities. > > Also note regarding your first paragraph, that specifying flags based on > kernel you are running on doesn’t help for the case discussed here. > As source QEMU is runn
Re: [Qemu-devel] Deprecation policy and build dependencies
On Wed, Jun 05, 2019 at 03:13:08PM -0300, Eduardo Habkost wrote: > On Wed, Jun 05, 2019 at 04:44:03PM +0100, Daniel P. Berrangé wrote: > [...] > > Thus to answer your python 2 question, we should ask which of our build > > targets cannot support python 3 ? > > > > Obviously we know the answer to that is RHEL-7. Except there is some > > fuzziness in there because it depends on what you define "RHEL-7" to > > be. There are several possible answers > > > > a. RHEL-7 covers only the stuff in the basic yum repos > > b. RHEL-7 covers packages in any yum repos shipped by Red Hat > > c. RHEL-7 covers packages in any yum repos shipped by Red Hat or EPEL > > d. RHEL-7 covers packages in any yum repo available for use > > with RHEL-7, provided by any vendor > > > > The platform support policy has not documented which of these possiblities > > we're targetting. > > > > If we consider it to mean (a), then there's no way to use py3 with RHEL-7. > > > > With (b), (c), or (d) it is possible to get py3 available on RHEL-7 by > > enabling suitable repos. > > > > Personally I think it would be fine for use to consider (b) or (c) to be > > our intended interpretation for platform support policy. > > (c) sounds like the best option, to me. Do we have any > reason to prefer (b) instead of (c)? Depends how flexible we want to be. Some enterprise organizations will not allow use of 3rd party repos, even EPEL, only permitting to consume software provided by Red Hat official repos. I'm not too bothered though as I doubt it will be a major problem with likely QEMU contributors. Those restrictive organizations are not likely to allow developers to be involved in upstream communities in the first place, nor consume releases direct from upstream. IOW, I think (c) is fine to allow maximum flexibility for upstream. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH] nvme: do not advertise support for unsupported arbitration mechanism
The device mistakenly reports that the Weighted Round Robin with Urgent Priority Class arbitration mechanism is supported. It is not. Signed-off-by: Klaus Birkelund Jensen --- hw/block/nvme.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 30e50f7a3853..415b4641d6b4 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1383,7 +1383,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) n->bar.cap = 0; NVME_CAP_SET_MQES(n->bar.cap, 0x7ff); NVME_CAP_SET_CQR(n->bar.cap, 1); -NVME_CAP_SET_AMS(n->bar.cap, 1); NVME_CAP_SET_TO(n->bar.cap, 0xf); NVME_CAP_SET_CSS(n->bar.cap, 1); NVME_CAP_SET_MPSMAX(n->bar.cap, 4); -- 2.21.0
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] docs/devel/build-system: Update an example
Le 29/05/2019 à 16:05, Philippe Mathieu-Daudé a écrit : > The default-configs/ example added in 717171bd2025 is no > more accurate since fa212a2b8b60 (and various further other > commits). > > The Kconfig build system is now in place. > Use the aarch64-softmmu config as example. > > Reviewed-by: Thomas Huth > Signed-off-by: Philippe Mathieu-Daudé > --- > v2: rephrased the sentence using Thomas suggestion > --- > docs/devel/build-system.txt | 19 +++ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt > index addd274eeb..41bd08ea3a 100644 > --- a/docs/devel/build-system.txt > +++ b/docs/devel/build-system.txt > @@ -413,18 +413,13 @@ context. > - default-configs/*.mak > > The files under default-configs/ control what emulated hardware is built > -into each QEMU system and userspace emulator targets. They merely > -contain a long list of config variable definitions. For example, > -default-configs/x86_64-softmmu.mak has: > - > - include sound.mak > - include usb.mak > - CONFIG_QXL=$(CONFIG_SPICE) > - CONFIG_VGA_ISA=y > - CONFIG_VGA_CIRRUS=y > - CONFIG_VMWARE_VGA=y > - CONFIG_VIRTIO_VGA=y > - ...snip... > +into each QEMU system and userspace emulator targets. They merely contain > +a list of config variable definitions like the machines that should be > +included. For example, default-configs/aarch64-softmmu.mak has: > + > + include arm-softmmu.mak > + CONFIG_XLNX_ZYNQMP_ARM=y > + CONFIG_XLNX_VERSAL=y > > These files rarely need changing unless new devices / hardware need to > be enabled for a particular system/userspace emulation target > Applied to my trivial-patches branch. Thanks, Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 01/10] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Since the BusState is accesible from the SCSIBus object,
> it is pointless to use qbus_reset_all_fn.
> Use qbus_reset_all() directly.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> v2: Use BUS() macro (Peter Maydell)
>
> One step toward removing qbus_reset_all_fn()
> ---
> hw/scsi/vmw_pvscsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 584b4be07e..c39e33fa35 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -440,7 +440,7 @@ static void
> pvscsi_reset_adapter(PVSCSIState *s)
> {
> s->resetting++;
> -qbus_reset_all_fn(&s->bus);
> +qbus_reset_all(BUS(&s->bus));
> s->resetting--;
> pvscsi_process_completion_queue(s);
> assert(QTAILQ_EMPTY(&s->pending_queue));
> @@ -848,7 +848,7 @@ pvscsi_on_cmd_reset_bus(PVSCSIState *s)
> trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_RESET_BUS");
>
> s->resetting++;
> -qbus_reset_all_fn(&s->bus);
> +qbus_reset_all(BUS(&s->bus));
> s->resetting--;
> return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 03/10] hw/pci-bridge: Use the QOM BUS() macro to access BusState.qbus
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a BusState with "s->bus.qbus",
> use the QOM prefered style: "BUS(&s->bus)".
>
> This patch was generated using the following Coccinelle script:
>
> // Use BUS() macros to access BusState.qbus
> @use_bus_macro_to_access_qbus@
> expression obj;
> identifier bus;
> @@
> -&obj->bus.qbus
> +BUS(&obj->bus)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/pci/pci_bridge.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index c6d9ded320..8d954885c0 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -273,7 +273,7 @@ void pci_bridge_write_config(PCIDevice *d,
> newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> /* Trigger hot reset on 0->1 transition. */
> -qbus_reset_all(&s->sec_bus.qbus);
> +qbus_reset_all(BUS(&s->sec_bus));
> }
> }
>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 04/10] hw/s390x/event-facility: Use the QOM BUS() macro to access BusState.qbus
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit : > Rather than looking inside the definition of a BusState with "s->bus.qbus", > use the QOM prefered style: "BUS(&s->bus)". > > This patch was generated using the following Coccinelle script: > > // Use BUS() macros to access BusState.qbus > @use_bus_macro_to_access_qbus@ > expression obj; > identifier bus; > @@ > -&obj->bus.qbus > +BUS(&obj->bus) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/s390x/event-facility.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index ee5b83448b..e574223a22 100644 > --- a/hw/s390x/event-facility.c > +++ b/hw/s390x/event-facility.c > @@ -466,12 +466,12 @@ static void init_event_facility(Object *obj) > new = object_new(TYPE_SCLP_QUIESCE); > object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL); > object_unref(new); > -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); > > new = object_new(TYPE_SCLP_CPU_HOTPLUG); > object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL); > object_unref(new); > -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); > /* the facility will automatically realize the devices via the bus */ > } > > Applied to my trivial-patches branch. Thanks, Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 07/10] hw/isa: Use the QOM DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit : > Rather than looking inside the definition of a DeviceState with > "s->qdev", use the QOM prefered style: "DEVICE(s)". > > This patch was generated using the following Coccinelle script: > > // Use DEVICE() macros to access DeviceState.qdev > @use_device_macro_to_access_qdev@ > expression obj; > identifier dev; > @@ > -&obj->dev.qdev > +DEVICE(obj) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/isa/lpc_ich9.c | 2 +- > hw/isa/vt82c686.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 031ee9cd93..35d17246e9 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -412,7 +412,7 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool > smm_enabled) > true); > } > > -ich9_lpc_reset(&lpc->d.qdev); > +ich9_lpc_reset(DEVICE(lpc)); > } > > /* APM */ > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index 85d0532dd5..d46754f61c 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -369,7 +369,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error > **errp) > pci_conf[0x90] = s->smb_io_base | 1; > pci_conf[0x91] = s->smb_io_base >> 8; > pci_conf[0xd2] = 0x90; > -pm_smbus_init(&s->dev.qdev, &s->smb, false); > +pm_smbus_init(DEVICE(s), &s->smb, false); > memory_region_add_subregion(get_system_io(), s->smb_io_base, &s->smb.io); > > apm_init(dev, &s->apm, NULL, s); > Applied to my trivial-patches branch. Thanks, Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 08/10] hw/usb-storage: Use the QOM DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit : > Rather than looking inside the definition of a DeviceState with > "s->qdev", use the QOM prefered style: "DEVICE(s)". > > This patch was generated using the following Coccinelle script: > > // Use DEVICE() macros to access DeviceState.qdev > @use_device_macro_to_access_qdev@ > expression obj; > identifier dev; > @@ > -&obj->dev.qdev > +DEVICE(obj) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/dev-storage.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c > index cd5551d94f..0e4e93ef16 100644 > --- a/hw/usb/dev-storage.c > +++ b/hw/usb/dev-storage.c > @@ -616,7 +616,7 @@ static void usb_msd_storage_realize(USBDevice *dev, Error > **errp) > * The hack is probably a bad idea. > */ > blk_ref(blk); > -blk_detach_dev(blk, &s->dev.qdev); > +blk_detach_dev(blk, DEVICE(s)); > s->conf.blk = NULL; > > usb_desc_create_serial(dev); > Applied to my trivial-patches branch. Thanks, Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 09/10] hw/vfio/pci: Use the QOM DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a DeviceState with
> "s->qdev", use the QOM prefered style: "DEVICE(s)".
>
> This patch was generated using the following Coccinelle script:
>
> // Use DEVICE() macros to access DeviceState.qdev
> @use_device_macro_to_access_qdev@
> expression obj;
> identifier dev;
> @@
> -&obj->dev.qdev
> +DEVICE(obj)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/vfio/pci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 8e555db12e..2a4091d216 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2717,7 +2717,7 @@ static void vfio_req_notifier_handler(void *opaque)
> return;
> }
>
> -qdev_unplug(&vdev->pdev.qdev, &err);
> +qdev_unplug(DEVICE(vdev), &err);
> if (err) {
> warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> }
> @@ -2839,7 +2839,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
> vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
> vdev->vbasedev.ops = &vfio_pci_ops;
> vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
> -vdev->vbasedev.dev = &vdev->pdev.qdev;
> +vdev->vbasedev.dev = DEVICE(vdev);
>
> tmp = g_strdup_printf("%s/iommu_group", vdev->vbasedev.sysfsdev);
> len = readlink(tmp, group_path, sizeof(group_path));
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 02/10] hw/scsi: Use the QOM BUS() macro to access BusState.qbus
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit : > Rather than looking inside the definition of a BusState with "s->bus.qbus", > use the QOM prefered style: "BUS(&s->bus)". > > This patch was generated using the following Coccinelle script: > > // Use BUS() macros to access BusState.qbus > @use_bus_macro_to_access_qbus@ > expression obj; > identifier bus; > @@ > -&obj->bus.qbus > +BUS(&obj->bus) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/scsi/lsi53c895a.c | 2 +- > hw/scsi/mptsas.c | 4 ++-- > hw/scsi/virtio-scsi.c | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Laurent Vivier
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 10/10] hw/watchdog/wdt_i6300esb: Use DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit : > Rather than looking inside the definition of a DeviceState with > "s->qdev", use the QOM prefered style: "DEVICE(s)". > > This patch was generated using the following Coccinelle script: > > // Use DEVICE() macros to access DeviceState.qdev > @use_device_macro_to_access_qdev@ > expression obj; > identifier dev; > @@ > -&obj->dev.qdev > +DEVICE(obj) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/watchdog/wdt_i6300esb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Laurent Vivier
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 06/10] hw/audio/ac97: Use the QOM DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a DeviceState with
> "s->qdev", use the QOM prefered style: "DEVICE(s)".
>
> This patch was generated using the following Coccinelle script
> (with a bit of manual fix-up, removing an extra space to please
> checkpatch.pl):
>
> // Use DEVICE() macros to access DeviceState.qdev
> @use_device_macro_to_access_qdev@
> expression obj;
> identifier dev;
> @@
> -&obj->dev.qdev
> +DEVICE(obj)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/audio/ac97.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> index 2265622d44..a4e8d99e77 100644
> --- a/hw/audio/ac97.c
> +++ b/hw/audio/ac97.c
> @@ -1388,7 +1388,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
> pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
> pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
> AUD_register_card ("ac97", &s->card);
> -ac97_on_reset (&s->dev.qdev);
> +ac97_on_reset(DEVICE(s));
> }
>
> static void ac97_exit(PCIDevice *dev)
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 05/10] hw/sd: Use the QOM BUS() macro to access BusState.qbus
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a BusState with "s->bus.qbus",
> use the QOM prefered style: "BUS(&s->bus)".
>
> This patch was generated using the following Coccinelle script:
>
> // Use BUS() macros to access BusState.qbus
> @use_bus_macro_to_access_qbus@
> expression obj;
> identifier bus;
> @@
> -&obj->bus.qbus
> +BUS(&obj->bus)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/sd/milkymist-memcard.c | 2 +-
> hw/sd/ssi-sd.c| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index df42aa1c54..dd1ba649d9 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -277,7 +277,7 @@ static void milkymist_memcard_realize(DeviceState *dev,
> Error **errp)
> /* FIXME use a qdev drive property instead of drive_get_next() */
> dinfo = drive_get_next(IF_SD);
> blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> -carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD);
> qdev_prop_set_drive(carddev, "drive", blk, &err);
> object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> if (err) {
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 623d0333e8..25e1009277 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -249,7 +249,7 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
> /* Create and plug in the sd card */
> /* FIXME use a qdev drive property instead of drive_get_next() */
> dinfo = drive_get_next(IF_SD);
> -carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD);
> if (dinfo) {
> qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo),
> &err);
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] Qemu baseline requirements/portability?
On Thu, Jun 06, 2019 at 07:30:08AM +0200, Markus Armbruster wrote: > "H. Peter Anvin" writes: > > > On 6/5/19 12:55 PM, H. Peter Anvin wrote: > >> Hi, > >> > >> I am writing some code I'm hoping will be able to make it into Qemu, but I > >> can't seem to find what the baseline portability requirements are. I'm > >> specifically wondering about newer POSIX features like openat(), which > >> seems > >> to be used in the 9p filesystem and nowhere else, and what version of glib > >> one > >> can rely on? > >> > > > > Specifically, I'm trying to satisfy a 10-year-old request by me and others > > to > > support composite initrd during Linux boot. > > Please peruse qemu-doc.texi @appendix Supported build platforms. If it > doesn't fully answer your question, ask for clarification here. And specificially on the glib version question, we currently mandate 2.40. If you try to use a glib API from a version newer than 2.40 we have things setup so that you should get a fatal compile error due to our use of #define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_40 If you do think it is useful to have an API from glib > 2.40 you can put some back compat logic in include/glib-compat.h so that you can use the newer API and gracefully degrade on older glib. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signalsy
On Wed, Jun 05, 2019 at 09:57:01PM +0100, Richard Henderson wrote:
> The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
> so we need to make sure that the value is 0 before clone,
> fork, or syscall return.
>
> The kernel sets btype for the signal handler as if for a call.
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/aarch64/cpu_loop.c | 7 +++
> linux-user/aarch64/signal.c | 10 --
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 2f2f63e3e8..1f68b13168 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -86,6 +86,13 @@ void cpu_loop(CPUARMState *env)
>
> switch (trapnr) {
> case EXCP_SWI:
> +/*
> + * The state of BTYPE on syscall entry is CONSTRAINED
> + * UNPREDICTABLE. The real kernel will need to tidy this up
> + * as well. Do this before syscalls so that the value is
> + * correct on return from syscall (especially clone & fork).
> + */
> +env->btype = 0;
Note, after discussion with the architects, I think Linux won't bother
sanitising this field in my next spin of the patches.
If the SVC (or HVC or SMC) sits in a PROT_BTI page, then it won't
execute anyway unless BTYPE is already 0: otherwise, a branch target
exception would be taken instead.
If the insn isn't in a PROT_BTI page, then BTYPE could be nonzero when
the SVC (etc.) exception is taken, but it also won't matter in that case
what BTYPE is on return, since branch target exceptions are never
generated on insns in non-guarded pages.
For this to make a difference:
a) the SVC be in a non-PROT_BTI page, just before a page boundary,
where the next page is a PROT_BTI page, so that the exception return
goes to the next page.
b) the SVC must be an mprotect() call or similar that enabled PROT_BTI
for the patch containing the SVC itself.
These are both silly things to do, and we probably don't care what
happens in such cases.
(Question: does qemu ever mprotect() the page containing PC? I'd hope
not...)
> ret = do_syscall(env,
> env->xregs[8],
> env->xregs[0],
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
> index f84a9cf28a..5605d404b3 100644
> --- a/linux-user/aarch64/signal.c
> +++ b/linux-user/aarch64/signal.c
> @@ -506,10 +506,16 @@ static void target_setup_frame(int usig, struct
> target_sigaction *ka,
> + offsetof(struct target_rt_frame_record, tramp);
> }
> env->xregs[0] = usig;
> -env->xregs[31] = frame_addr;
> env->xregs[29] = frame_addr + fr_ofs;
> -env->pc = ka->_sa_handler;
> env->xregs[30] = return_addr;
> +env->xregs[31] = frame_addr;
> +env->pc = ka->_sa_handler;
> +
> +/* Invoke the signal handler as if by indirect call. */
> +if (cpu_isar_feature(aa64_bti, arm_env_get_cpu(env))) {
> +env->btype = 2;
> +}
> +
Ack. I had a simple test for this for native userspace, and it seems to
work as desired -- so I don't expect to change it.
Cheers
---Dave
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 02/10] hw/scsi: Use the QOM BUS() macro to access BusState.qbus
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a BusState with "s->bus.qbus",
> use the QOM prefered style: "BUS(&s->bus)".
>
> This patch was generated using the following Coccinelle script:
>
> // Use BUS() macros to access BusState.qbus
> @use_bus_macro_to_access_qbus@
> expression obj;
> identifier bus;
> @@
> -&obj->bus.qbus
> +BUS(&obj->bus)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/scsi/lsi53c895a.c | 2 +-
> hw/scsi/mptsas.c | 4 ++--
> hw/scsi/virtio-scsi.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index da7239d94f..a8b7a199f9 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -1860,7 +1860,7 @@ static void lsi_reg_writeb(LSIState *s, int offset,
> uint8_t val)
> }
> if (val & LSI_SCNTL1_RST) {
> if (!(s->sstat0 & LSI_SSTAT0_RST)) {
> -qbus_reset_all(&s->bus.qbus);
> +qbus_reset_all(BUS(&s->bus));
> s->sstat0 |= LSI_SSTAT0_RST;
> lsi_script_scsi_interrupt(s, LSI_SIST0_RST, 0);
> }
> diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
> index 929404fb48..e800683e91 100644
> --- a/hw/scsi/mptsas.c
> +++ b/hw/scsi/mptsas.c
> @@ -540,7 +540,7 @@ reply_maybe_async:
> break;
>
> case MPI_SCSITASKMGMT_TASKTYPE_RESET_BUS:
> -qbus_reset_all(&s->bus.qbus);
> +qbus_reset_all(BUS(&s->bus));
> break;
>
> default:
> @@ -803,7 +803,7 @@ static void mptsas_soft_reset(MPTSASState *s)
> s->intr_mask = MPI_HIM_DIM | MPI_HIM_RIM;
> mptsas_update_interrupt(s);
>
> -qbus_reset_all(&s->bus.qbus);
> +qbus_reset_all(BUS(&s->bus));
> s->intr_status = 0;
> s->intr_mask = save_mask;
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 839f120256..2a71b78e22 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -696,7 +696,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
>
> assert(!s->dataplane_started);
> s->resetting++;
> -qbus_reset_all(&s->bus.qbus);
> +qbus_reset_all(BUS(&s->bus));
> s->resetting--;
>
> vs->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2 10/10] hw/watchdog/wdt_i6300esb: Use DEVICE() macro to access DeviceState.qdev
Le 28/05/2019 à 18:40, Philippe Mathieu-Daudé a écrit :
> Rather than looking inside the definition of a DeviceState with
> "s->qdev", use the QOM prefered style: "DEVICE(s)".
>
> This patch was generated using the following Coccinelle script:
>
> // Use DEVICE() macros to access DeviceState.qdev
> @use_device_macro_to_access_qdev@
> expression obj;
> identifier dev;
> @@
> -&obj->dev.qdev
> +DEVICE(obj)
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/watchdog/wdt_i6300esb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> index 1c6eddf86a..f2d1e86526 100644
> --- a/hw/watchdog/wdt_i6300esb.c
> +++ b/hw/watchdog/wdt_i6300esb.c
> @@ -200,7 +200,7 @@ static void i6300esb_timer_expired(void *vp)
> if (d->reboot_enabled) {
> d->previous_reboot_flag = 1;
> watchdog_perform_action(); /* This reboots, exits, etc */
> -i6300esb_reset(&d->dev.qdev);
> +i6300esb_reset(DEVICE(d));
> }
>
> /* In "free running mode" we start stage 1 again. */
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [Qemu-devel] [Qemu-trivial] [PATCH] scripts/qmp: fix deleted link in qemu-ga-client script
Le 07/05/2019 à 20:41, Rami Rosen a écrit : > This patch fixes a deleted link in qemu-ga-client script. > According to the old link, the page moved to > https://wiki.qemu.org/Features/QAPI/GuestAgent. > > Signed-off-by: Rami Rosen > --- > scripts/qmp/qemu-ga-client | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client > index 30cf8a9..50efc21 100755 > --- a/scripts/qmp/qemu-ga-client > +++ b/scripts/qmp/qemu-ga-client > @@ -33,7 +33,7 @@ > # $ qemu-ga-client fsfreeze freeze > # 2 filesystems frozen > # > -# See also: https://wiki.qemu.org/Features/QAPI/GuestAgent > +# See also: https://wiki.qemu.org/Features/GuestAgent > # > > from __future__ import print_function > Please re-send your patch cc'ing qemu-devel@nongnu.org. Thanks, LAurent
Re: [Qemu-devel] [PATCH v2 2/2] blockjob: use blk_new_pinned in block_job_create
Am 05.06.2019 um 19:16 hat Vladimir Sementsov-Ogievskiy geschrieben: > 05.06.2019 20:11, Kevin Wolf wrote: > > Am 05.06.2019 um 14:32 hat Vladimir Sementsov-Ogievskiy geschrieben: > >> child_role job already has .stay_at_node=true, so on bdrv_replace_node > >> operation these child are unchanged. Make block job blk behave in same > >> manner, to avoid inconsistent intermediate graph states and workarounds > >> like in mirror. > >> > >> Signed-off-by: Vladimir Sementsov-Ogievskiy > > > > This feels dangerous. It does what you want it to do if the only graph > > change below the BlockBackend is the one in mirror_exit_common. But the > > user could also take a snapshot, or in the future hopefully insert a > > filter node, and you would then want the BlockBackend to move. > > > > To be honest, even BdrvChildRole.stay_at_node is a bit of a hack. But at > > least it's only used for permissions and not for the actual data flow. > > Hmm. Than, may be just add a parameter to bdrv_replace_node, which parents > to ignore? Would it work? I would have to think a bit more about it, but it does sound safer. Or we take a step back and ask why it's even a problem for the mirror block job if the BlockBackend is moved to a different node. The main reason I see is because of bs->job that is set for the root node of the BlockBackend and needs to be unset for the same node. Maybe we can just finally get rid of bs->job? It doesn't have many users any more. Kevin
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
> On 6 Jun 2019, at 12:23, Dr. David Alan Gilbert wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> >> >>> On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert wrote: >>> >>> * Liran Alon (liran.a...@oracle.com) wrote: Hi, Looking at QEMU source code, I am puzzled regarding how migration backwards compatibility is preserved regarding X86CPU. As I understand it, fields that are based on KVM capabilities and guest runtime usage are defined in VMState subsections in order to not send them if not necessary. This is done such that in case they are not needed and we migrate to an old QEMU which don’t support loading this state, migration will still succeed (As .needed() method will return false and therefore this state won’t be sent as part of migration stream). Furthermore, in case .needed() returns true and old QEMU don’t support loading this state, migration fails. As it should because we are aware that guest state is not going to be restored properly on destination. I’m puzzled about what will happen in the following scenario: 1) Source is running new QEMU with new KVM that supports save of some VMState subsection. 2) Destination is running new QEMU that supports load this state but with old kernel that doesn’t know how to load this state. I would have expected in this case that if source .needed() returns true, then migration will fail because of lack of support in destination kernel. However, it seems from current QEMU code that this will actually succeed in many cases. For example, if msr_smi_count is sent as part of migration stream (See vmstate_msr_smi_count) and destination have has_msr_smi_count==false, then destination will succeed loading migration stream but kvm_put_msrs() will actually ignore env->msr_smi_count and will successfully load guest state. Therefore, migration will succeed even though it should have failed… It seems to me that QEMU should have for every such VMState subsection, a .post_load() method that verifies that relevant capability is supported by kernel and otherwise fail migration. What do you think? Should I really create a patch to modify all these CPUX86 VMState subsections to behave like this? >>> >>> I don't know the x86 specific side that much; but from my migration side >>> the answer should mostly be through machine types - indeed for smi-count >>> there's a property 'x-migrate-smi-count' which is off for machine types >>> pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old >>> kernel you should stick to the old machine types. >>> >>> There's nothing guarding running the new machine type on old-kernels; >>> and arguably we should have a check at startup that complains if >>> your kernel is missing something the machine type uses. >>> However, that would mean that people running with -M pc would fail >>> on old kernels. >>> >>> A post-load is also a valid check; but one question is whether, >>> for a particular register, the pain is worth it - it depends on the >>> symptom that the missing state causes. If it's minor then you might >>> conclude it's not worth a failed migration; if it's a hung or >>> corrupt guest then yes it is. Certainly a warning printed is worth >>> it. >>> >>> Dave >> >> I think we should have flags that allow user to specify which VMState >> subsections user explicitly allow to avoid restore even though they are >> required to fully restore guest state. >> But it seems to me that the behaviour should be to always fail migration in >> case we load a VMState subsections that we are unable to restore unless user >> explicitly specified this is ok >> for this specific subsection. >> Therefore, it seems that for every VMState subsection that it’s restore is >> based on kernel capability we should: >> 1) Have a user-controllable flag (which is also tied to machine-type?) to >> explicitly allow avoid restoring this state if cannot. Default should be >> “false”. >> 2) Have a .post_load() method that verifies we have required kernel >> capability to restore this state, unless flag (1) was specified as “true”. > > This seems a lot of flags; users aren't going to know what to do with > all of them; I don't see what will set/control them. True but I think users will want to specify only for a handful of VMState subsections that it is OK to not restore them even thought hey are deemed needed by source QEMU. We can create flags only for those VMState subsections. User should set these flags explicitly on QEMU command-line. As a “-cpu” property? I don’t think these flags should be tied to machine-type. > >> Note that above mentioned flags is different than flags such as >> “x-migrate-smi-count”. >> The purpose of “x-migrate-smi-count” flag is to avoid sending the VMState >
Re: [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes
On Wed, Jun 05, 2019 at 09:57:05PM +0100, Richard Henderson wrote:
> For aarch64, this includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI bit,
> which indicates that the image should be mapped with guarded pages.
Heads-up: for arm64 I plan to move to making PT_GNU_PROPERTY
authoritiative for ELF on linux: if this is present, we use it to find
the note directly and ignore any other notes; if there is no
PT_GNU_PROPERTY entry then we assume there is no NT_GNU_PROPERTY_TYPE_0
note.
This is not quite decided yet, but to avoid fragmentation I'd prefer
qemu and Linux apply the same policy -- I'll keep you in the loop about
the decision.
I think you can reasonable upstream this patch in qemu and then
subsequently make it stricter without an ABI break. Upstream GNU ld
generates this entry today.
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/elfload.c | 83 +++-
> 1 file changed, 75 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index a57b7049dd..1a12c60a33 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2253,7 +2253,7 @@ static void load_elf_image(const char *image_name, int
> image_fd,
> struct elfhdr *ehdr = (struct elfhdr *)bprm_buf;
> struct elf_phdr *phdr;
> abi_ulong load_addr, load_bias, loaddr, hiaddr, error;
> -int i, retval;
> +int i, retval, prot_exec = PROT_EXEC;
> const char *errmsg;
>
> /* First of all, some simple consistency checks */
> @@ -2288,17 +2288,78 @@ static void load_elf_image(const char *image_name,
> int image_fd,
> loaddr = -1, hiaddr = 0;
> info->alignment = 0;
> for (i = 0; i < ehdr->e_phnum; ++i) {
> -if (phdr[i].p_type == PT_LOAD) {
> -abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
> +struct elf_phdr *eppnt = phdr + i;
> +
> +if (eppnt->p_type == PT_LOAD) {
> +abi_ulong a = eppnt->p_vaddr - eppnt->p_offset;
> if (a < loaddr) {
> loaddr = a;
> }
> -a = phdr[i].p_vaddr + phdr[i].p_memsz;
> +a = eppnt->p_vaddr + eppnt->p_memsz;
> if (a > hiaddr) {
> hiaddr = a;
> }
> ++info->nsegs;
> -info->alignment |= phdr[i].p_align;
> +info->alignment |= eppnt->p_align;
> +} else if (eppnt->p_type == PT_NOTE) {
> +#ifdef TARGET_AARCH64
> +/*
> + * Process NT_GNU_PROPERTY_TYPE_0.
> + *
> + * TODO: The only item that is AArch64 specific is the
> + * GNU_PROPERTY_AARCH64_FEATURE_1_AND processing at the end.
> + * If we were to ever process GNU_PROPERTY_X86_*, all of the
> + * code through checking the gnu0 magic number is sharable.
> + * But for now, since this *is* only used by AArch64, don't
> + * process the note elsewhere.
> + */
> +const uint32_t gnu0_magic = const_le32('G' | 'N' << 8 | 'U' <<
> 16);
> +uint32_t note[7];
> +
> +/*
> + * The note contents are 7 words, but depending on LP64 vs ILP32
> + * there may be an 8th padding word at the end. Check for and
> + * read the minimum size. Further checks below will validate
> + * that the sizes of everything involved are as we expect.
> + */
> +if (eppnt->p_filesz < sizeof(note)) {
> +continue;
> +}
> +if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> +memcpy(note, bprm_buf + eppnt->p_offset, sizeof(note));
> +} else {
> +retval = pread(image_fd, note, sizeof(note),
> eppnt->p_offset);
> +if (retval != sizeof(note)) {
> +goto exit_perror;
> +}
> +}
Can we police that the segment alignment matches the ELF class (i.e., 8
for 64-bit, 4 for 32-bit)?
hjl specifies this, but it's controversial and sometimes missed --
there's a bug right now in GNU ld where --force-bti generates a note
with alignment 1.
Due to the high chance of screwing this up, it'd be good to police it
wherever appropriate.
> +#ifdef BSWAP_NEEDED
> +for (i = 0; i < ARRAY_SIZE(note); ++i) {
> +bswap32s(note + i);
> +}
> +#endif
> +/*
> + * Check that this is a NT_GNU_PROPERTY_TYPE_0 note.
> + * Again, descsz includes padding. Full size validation
> + * awaits checking the final payload.
> + */
> +if (note[0] != 4 || /* namesz */
> +note[1] < 12 || /* descsz */
> +note[2] != NT_GNU_PROPERTY_TYPE_0 || /* type */
> +note[3] != gnu0_magic) { /* name */
> +continue;
> +}
> +
[Qemu-devel] [PATCH] qapi: InitSocketAddress: add keepalive option
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
Hi all!
This is a continuation of "[PATCH v2 0/2] nbd: enable keepalive", but
it's a try from another side, so almost nothing common with v2.
qapi/sockets.json | 5 -
util/qemu-sockets.c | 13 +
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/qapi/sockets.json b/qapi/sockets.json
index fc81d8d5e8..aefa024051 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -53,6 +53,8 @@
#
# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
#
+# @keepalive: enable keepalive when connecting to this socket (Since 4.1)
+#
# Since: 1.3
##
{ 'struct': 'InetSocketAddress',
@@ -61,7 +63,8 @@
'*numeric': 'bool',
'*to': 'uint16',
'*ipv4': 'bool',
-'*ipv6': 'bool' } }
+'*ipv6': 'bool',
+'*keepalive': 'bool' } }
##
# @UnixSocketAddress:
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8850a280a8..d2cd2a9d4f 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -457,6 +457,19 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error
**errp)
}
freeaddrinfo(res);
+
+if (saddr->keepalive) {
+int val = 1;
+int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
+ &val, sizeof(val));
+
+if (ret < 0) {
+error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
+close(sock);
+return -1;
+}
+}
+
return sock;
}
--
2.18.0
[Qemu-devel] [PATCH 2/8] target/ppc: Optimize emulation of vsl and vsr instructions
Optimization of altivec instructions vsl and vsr(Vector Shift Left/Rigt).
Perform shift operation (left and right respectively) on 128 bit value of
register vA by value specified in bits 125-127 of register vB. Lowest 3
bits in each byte element of register vB must be identical or result is
undefined.
For vsl instruction we do this by first saving bits 125-127
of register vB in variable sh. Then we save highest sh bits
of lower doubleword element of register vA in variable shifted,
so we don't lose those bits when we perform shift operation on
lower doubleword element of register vA, which is our next
step. After shifting lower doubleword element we perform shift
operation on higher doubleword element of vA and replace
lowest sh bits(that are now 0) with bits saved in shifted.
For vsr instruction we do this by first saving bits 125-127
of register vB in variable sh. Then we save lowest sh bits
of higher doubleword element of register vA in variable shifted,
so we don't lose those bits when we perform shift operation on
higher doubleword element of register vA, which is our next step.
After shifting higher doubleword element we perform shift
operation on lower doubleword element of vA and replace
highest sh bits(that are now 0) with bits saved in shifted.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 101 +++-
1 file changed, 99 insertions(+), 2 deletions(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 140bb05..6bd072a 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -592,6 +592,103 @@ static void trans_lvsr(DisasContext *ctx)
tcg_temp_free(EA);
}
+/*
+ * vsl VRT,VRA,VRB - Vector Shift Left
+ *
+ * Shifting left 128 bit value of vA by value specified in bits 125-127 of vB.
+ * Lowest 3 bits in each byte element of register vB must be identical or
+ * result is undefined.
+ */
+static void trans_vsl(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VA = rA(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avrA = tcg_temp_new_i64();
+TCGv_i64 avrB = tcg_temp_new_i64();
+TCGv_i64 sh = tcg_temp_new_i64();
+TCGv_i64 shifted = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+
+/* Place bits 125-127 of vB in sh. */
+get_avr64(avrB, VB, false);
+tcg_gen_andi_i64(sh, avrB, 0x07ULL);
+
+/*
+ * Save highest sh bits of lower doubleword element of vA in variable
+ * shifted and perform shift on lower doubleword.
+ */
+get_avr64(avrA, VA, false);
+tcg_gen_subi_i64(tmp, sh, 64);
+tcg_gen_neg_i64(tmp, tmp);
+tcg_gen_shr_i64(shifted, avrA, tmp);
+tcg_gen_shl_i64(avrA, avrA, sh);
+set_avr64(VT, avrA, false);
+
+/*
+ * Perform shift on higher doubleword element of vA and replace lowest
+ * sh bits with shifted.
+ */
+get_avr64(avrA, VA, true);
+tcg_gen_shl_i64(avrA, avrA, sh);
+tcg_gen_or_i64(avrA, avrA, shifted);
+set_avr64(VT, avrA, true);
+
+tcg_temp_free_i64(avrA);
+tcg_temp_free_i64(avrB);
+tcg_temp_free_i64(sh);
+tcg_temp_free_i64(shifted);
+tcg_temp_free_i64(tmp);
+}
+
+/*
+ * vsr VRT,VRA,VRB - Vector Shift Right
+ *
+ * Shifting right 128 bit value of vA by value specified in bits 125-127 of vB.
+ * Lowest 3 bits in each byte element of register vB must be identical or
+ * result is undefined.
+ */
+static void trans_vsr(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VA = rA(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avrA = tcg_temp_new_i64();
+TCGv_i64 avrB = tcg_temp_new_i64();
+TCGv_i64 sh = tcg_temp_new_i64();
+TCGv_i64 shifted = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+
+/* Place bits 125-127 of vB in sh. */
+get_avr64(avrB, VB, false);
+tcg_gen_andi_i64(sh, avrB, 0x07ULL);
+
+/*
+ * Save lowest sh bits of higher doubleword element of vA in variable
+ * shifted and perform shift on higher doubleword.
+ */
+get_avr64(avrA, VA, true);
+tcg_gen_subi_i64(tmp, sh, 64);
+tcg_gen_neg_i64(tmp, tmp);
+tcg_gen_shl_i64(shifted, avrA, tmp);
+tcg_gen_shr_i64(avrA, avrA, sh);
+set_avr64(VT, avrA, true);
+/*
+ * Perform shift on lower doubleword element of vA and replace highest
+ * sh bits with shifted.
+ */
+get_avr64(avrA, VA, false);
+tcg_gen_shr_i64(avrA, avrA, sh);
+tcg_gen_or_i64(avrA, avrA, shifted);
+set_avr64(VT, avrA, false);
+
+tcg_temp_free_i64(avrA);
+tcg_temp_free_i64(avrB);
+tcg_temp_free_i64(sh);
+tcg_temp_free_i64(shifted);
+tcg_temp_free_i64(tmp);
+}
+
GEN_VXFORM(vmuloub, 4, 0);
GEN_VXFORM(vmulouh, 4, 1);
GEN_VXFORM(vmulouw, 4, 2);
@@ -699,11 +796,11 @@ GEN_VXFORM(vrld, 2, 3);
GEN_VXFORM(vrldmi, 2, 3);
GEN_VXFORM_DUAL(vrld, PPC_NONE, PPC2_ALTIVEC_207, \
vrldmi, PPC_NONE, PPC2_ISA300)
-GEN_VXFORM(vsl, 2, 7);
+GE
[Qemu-devel] [PATCH 1/8] target/ppc: Optimize emulation of lvsl and lvsr instructions
Adding simple macro that is calling tcg implementation of appropriate
instruction if altivec support is active.
Optimization of altivec instruction lvsl (Load Vector for Shift Left).
Place bytes sh:sh+15 of value 0x00 || 0x01 || 0x02 || ... || 0x1E || 0x1F
in destination register. Sh is calculated by adding 2 source registers and
getting bits 60-63 of result.
First we place bits [28-31] of EA to variable sh. After that
we create bytes sh:(sh+7) of X(from description) in for loop
(by incrementing sh in each iteration and placing it in
appropriate byte of variable result) and save them in higher
doubleword element of vD. We repeat this once again for lower
doubleword element of vD by creating bytes (sh+8):(sh+15) in
a for loop and saving result.
Optimization of altivec instruction lvsr (Load Vector for Shift Right).
Place bytes 16-sh:31-sh of value 0x00 || 0x01 || 0x02 || ... || 0x1E ||
0x1F in destination register. Sh is calculated by adding 2 source
registers and getting bits 60-63 of result.
First we place bits [28-31] of EA to variable sh. After that
we create bytes (16-sh):(23-sh) of X(from description) in for loop
(by incrementing sh in each iteration and placing it in
appropriate byte of variable result) and save them in higher
doubleword element of vD. We repeat this once again for lower
doubleword element of vD by creating bytes (24-sh):(32-sh) in
a for loop and saving result.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 143
1 file changed, 111 insertions(+), 32 deletions(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index bd3ff40..140bb05 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -142,38 +142,6 @@ GEN_VR_STVE(bx, 0x07, 0x04, 1);
GEN_VR_STVE(hx, 0x07, 0x05, 2);
GEN_VR_STVE(wx, 0x07, 0x06, 4);
-static void gen_lvsl(DisasContext *ctx)
-{
-TCGv_ptr rd;
-TCGv EA;
-if (unlikely(!ctx->altivec_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VPU);
-return;
-}
-EA = tcg_temp_new();
-gen_addr_reg_index(ctx, EA);
-rd = gen_avr_ptr(rD(ctx->opcode));
-gen_helper_lvsl(rd, EA);
-tcg_temp_free(EA);
-tcg_temp_free_ptr(rd);
-}
-
-static void gen_lvsr(DisasContext *ctx)
-{
-TCGv_ptr rd;
-TCGv EA;
-if (unlikely(!ctx->altivec_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VPU);
-return;
-}
-EA = tcg_temp_new();
-gen_addr_reg_index(ctx, EA);
-rd = gen_avr_ptr(rD(ctx->opcode));
-gen_helper_lvsr(rd, EA);
-tcg_temp_free(EA);
-tcg_temp_free_ptr(rd);
-}
-
static void gen_mfvscr(DisasContext *ctx)
{
TCGv_i32 t;
@@ -316,6 +284,16 @@ static void glue(gen_, name)(DisasContext *ctx)
\
tcg_temp_free_ptr(rd); \
}
+#define GEN_VXFORM_TRANS(name, opc2, opc3) \
+static void glue(gen_, name)(DisasContext *ctx) \
+{ \
+if (unlikely(!ctx->altivec_enabled)) { \
+gen_exception(ctx, POWERPC_EXCP_VPU); \
+return; \
+} \
+trans_##name(ctx); \
+}
+
#define GEN_VXFORM_ENV(name, opc2, opc3)\
static void glue(gen_, name)(DisasContext *ctx) \
{ \
@@ -515,6 +493,105 @@ static void gen_vmrgow(DisasContext *ctx)
tcg_temp_free_i64(avr);
}
+/*
+ * lvsl VRT,RA,RB - Load Vector for Shift Left
+ *
+ * Let the EA be the sum (rA|0)+(rB). Let sh=EA[28???31].
+ * Let X be the 32-byte value 0x00 || 0x01 || 0x02 || ... || 0x1E || 0x1F.
+ * Bytes sh:sh+15 of X are placed into vD.
+ */
+static void trans_lvsl(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+TCGv_i64 result = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 sh = tcg_temp_new_i64();
+TCGv_i64 EA = tcg_temp_new();
+int i;
+
+/* Get sh(from description) by anding EA with 0xf. */
+gen_addr_reg_index(ctx, EA);
+tcg_gen_andi_i64(sh, EA, 0xfULL);
+/*
+ * Create bytes sh:sh+7 of X(from description) and place them in
+ * higher doubleword of vD.
+ */
+tcg_gen_addi_i64(result, sh, 7);
+for (i = 7; i >= 1; i--) {
+tcg_gen_shli_i64(tmp, sh, i * 8);
+tcg_gen_or_i64(result, result, tmp);
+tcg_gen_addi_i64(sh, sh, 1);
+}
+set_avr64(VT, result, true);
+/*
+ * Create bytes sh+8:sh+15 of X(from description) and place them in
+ * lower doubleword of vD.
+ */
+tcg_gen_addi_i64(result, sh, 8);
+for (i = 7; i >= 1; i--) {
+
[Qemu-devel] [PATCH 7/8] target/ppc: Optimize emulation of vclzh and vclzb instructions
Optimize Altivec instruction vclzh (Vector Count Leading Zeros Halfword).
This instruction counts the number of leading zeros of each halfword element
in source register and places result in the appropriate halfword element of
destination register.
In each iteration of outer for loop we perform count operation on one
doubleword elements of source register vB. In first iteration we place
higher doubleword element of vB in variable avr, then we perform count
for every halfword element using tcg_gen_clzi_i64. Since it counts
leading zeros on 64 bit lenght, we have to move ith byte element to
highest 16 bits of tmp, or it with mask(so we get all ones in lowest
48 bits), then perform tcg_gen_clzi_i64 and move it's result in
appropriate halfword element of result. We do this in inner for loop.
After operation is finished we save result in appropriate doubleword
element of destination register vD. We repeat this once again for
lower doubleword element of vB.
Optimize Altivec instruction vclzb (Vector Count Leading Zeros Byte).
This instruction counts the number of leading zeros of each byte element
in source register and places result in the appropriate byte element of
destination register.
In each iteration of outer for loop we perform count operation on one
doubleword elements of source register vB. In first iteration we place
higher doubleword element of vB in variable avr, then we perform count
for every byte element using tcg_gen_clzi_i64. Since it counts leading
zeros on 64 bit lenght, we have to move ith byte element to highest 8
bits of variable tmp, or it with mask(so we get all ones in lowest 56
bits), then perform tcg_gen_clzi_i64 and move it's result in appropriate
byte element of result. We do this in inner for loop. After operation is
finished we save result in appropriate doubleword element of destination
register vD. We repeat this once again for lower doubleword element of
vB.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 122 +++-
1 file changed, 120 insertions(+), 2 deletions(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 7689739..8535a31 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -878,6 +878,124 @@ static void trans_vgbbd(DisasContext *ctx)
}
/*
+ * vclzb VRT,VRB - Vector Count Leading Zeros Byte
+ *
+ * Counting the number of leading zero bits of each byte element in source
+ * register and placing result in appropriate byte element of destination
+ * register.
+ */
+static void trans_vclzb(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avr = tcg_temp_new_i64();
+TCGv_i64 result = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 mask = tcg_const_i64(0xffULL);
+int i, j;
+
+for (i = 0; i < 2; i++) {
+if (i == 0) {
+/* Get high doubleword of vB in avr. */
+get_avr64(avr, VB, true);
+} else {
+/* Get low doubleword of vB in avr. */
+get_avr64(avr, VB, false);
+}
+/*
+ * Perform count for every byte element using tcg_gen_clzi_i64.
+ * Since it counts leading zeros on 64 bit lenght, we have to move
+ * ith byte element to highest 8 bits of tmp, or it with mask(so we get
+ * all ones in lowest 56 bits), then perform tcg_gen_clzi_i64 and move
+ * it's result in appropriate byte element of result.
+ */
+tcg_gen_shli_i64(tmp, avr, 56);
+tcg_gen_or_i64(tmp, tmp, mask);
+tcg_gen_clzi_i64(result, tmp, 64);
+for (j = 1; j < 7; j++) {
+tcg_gen_shli_i64(tmp, avr, (7 - j) * 8);
+tcg_gen_or_i64(tmp, tmp, mask);
+tcg_gen_clzi_i64(tmp, tmp, 64);
+tcg_gen_deposit_i64(result, result, tmp, j * 8, 8);
+}
+tcg_gen_or_i64(tmp, avr, mask);
+tcg_gen_clzi_i64(tmp, tmp, 64);
+tcg_gen_deposit_i64(result, result, tmp, 56, 8);
+if (i == 0) {
+/* Place result in high doubleword element of vD. */
+set_avr64(VT, result, true);
+} else {
+/* Place result in low doubleword element of vD. */
+set_avr64(VT, result, false);
+}
+}
+
+tcg_temp_free_i64(avr);
+tcg_temp_free_i64(result);
+tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(mask);
+}
+
+/*
+ * vclzh VRT,VRB - Vector Count Leading Zeros Halfword
+ *
+ * Counting the number of leading zero bits of each halfword element in source
+ * register and placing result in appropriate halfword element of destination
+ * register.
+ */
+static void trans_vclzh(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avr = tcg_temp_new_i64();
+TCGv_i64 result = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 mask = tcg_const_i64(
[Qemu-devel] [PATCH 0/8] Optimize emulation of ten Altivec instructions: lvsl,
This series buils up on and complements recent work of Thomas Murta, Mark Cave-Ayland and Richard Henderson in the same area. It is based on devising TCG translation implementation for selected instructions rather than using helpers. The selected instructions are most of the time idiosyncratic to ppc platform, so relatively complex TCG translation (without direct mapping to host instruction that is not possible in these cases) seems to be the best option, and that approach is presented in this series. The performance improvements are significant in all cases. Stefan Brankovic (8): target/ppc: Optimize emulation of lvsl and lvsr instructions target/ppc: Optimize emulation of vsl and vsr instructions target/ppc: Optimize emulation of vpkpx instruction target/ppc: Optimize emulation of vgbbd instruction target/ppc: Optimize emulation of vclzd instruction target/ppc: Optimize emulation of vclzw instruction target/ppc: Optimize emulation of vclzh and vclzb instructions target/ppc: Refactor emulation of vmrgew and vmrgow instructions target/ppc/translate/vmx-impl.inc.c | 705 1 file changed, 636 insertions(+), 69 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH 4/8] target/ppc: Optimize emulation of vgbbd instruction
Optimize altivec instruction vgbbd (Vector Gather Bits by Bytes by Doubleword)
All ith bits (i in range 1 to 8) of each byte of doubleword element in
source register are concatenated and placed into ith byte of appropriate
doubleword element in destination register.
Following solution is done for every doubleword element of source register
(placed in shifted variable):
We gather bits in 2x8 iterations.
In first iteration bit 1 of byte 1, bit 2 of byte 2,... bit 8 of byte 8 are
in their final spots so we just and avr with mask. For every next iteration,
we have to shift right both shifted(7 places) and mask(8 places), so we get
bit 1 of byte 2, bit 2 of byte 3.. bit 7 of byte 8 in right places so we and
shifted with new value of mask... After first 8 iteration(first for loop) we
have all first bits in their final place all second bits but second bit from
eight byte in their place,... only 1 eight bit from eight byte is in it's
place), so we and result1 with mask1 to save those bits that are at right
place and save them in result1. In second loop we do all operations
symetrical, so we get other half of bits on their final spots, and save
result in result2. Or of result1 and result2 is placed in appropriate
doubleword element of vD. We repeat this 2 times.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 99 -
1 file changed, 98 insertions(+), 1 deletion(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 87f69dc..010f337 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -780,6 +780,103 @@ static void trans_vsr(DisasContext *ctx)
tcg_temp_free_i64(tmp);
}
+/*
+ * vgbbd VRT,VRB - Vector Gather Bits by Bytes by Doubleword
+ *
+ * All ith bits (i in range 1 to 8) of each byte of doubleword element in
source
+ * register are concatenated and placed into ith byte of appropriate doubleword
+ * element in destination register.
+ *
+ * Following solution is done for every doubleword element of source register
+ * (placed in shifted variable):
+ * We gather bits in 2x8 iterations.
+ * In first iteration bit 1 of byte 1, bit 2 of byte 2,... bit 8 of byte 8 are
+ * in their final spots so we just and avr with mask. For every next iteration,
+ * we have to shift right both shifted(7 places) and mask(8 places), so we get
+ * bit 1 of byte 2, bit 2 of byte 3.. bit 7 of byte 8 in right places so we and
+ * shifted with new value of mask... After first 8 iteration(first for loop) we
+ * have all first bits in their final place all second bits but second bit from
+ * eight byte in their place,... only 1 eight bit from eight byte is in it's
+ * place), so we and result1 with mask1 to save those bits that are at right
+ * place and save them in result1. In second loop we do all operations
+ * symetrical, so we get other half of bits on their final spots, and save
+ * result in result2. Or of result1 and result2 is placed in appropriate
+ * doubleword element of vD. We repeat this 2 times.
+ */
+static void trans_vgbbd(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 avr = tcg_temp_new_i64();
+TCGv_i64 shifted = tcg_temp_new_i64();
+TCGv_i64 result1 = tcg_temp_new_i64();
+TCGv_i64 result2 = tcg_temp_new_i64();
+uint64_t mask = 0x8040201008040201ULL;
+uint64_t mask1 = 0x80c0e0f0f8fcfeffULL;
+uint64_t mask2 = 0x7f3f1f0f07030100ULL;
+int i;
+
+get_avr64(avr, VB, true);
+tcg_gen_movi_i64(result1, 0x0ULL);
+tcg_gen_mov_i64(shifted, avr);
+for (i = 0; i < 8; i++) {
+tcg_gen_andi_i64(tmp, shifted, mask);
+tcg_gen_or_i64(result1, result1, tmp);
+
+tcg_gen_shri_i64(shifted, shifted, 7);
+mask = mask >> 8;
+}
+tcg_gen_andi_i64(result1, result1, mask1);
+
+mask = 0x8040201008040201ULL;
+tcg_gen_movi_i64(result2, 0x0ULL);
+for (i = 0; i < 8; i++) {
+tcg_gen_andi_i64(tmp, avr, mask);
+tcg_gen_or_i64(result2, result2, tmp);
+
+tcg_gen_shli_i64(avr, avr, 7);
+mask = mask << 8;
+}
+tcg_gen_andi_i64(result2, result2, mask2);
+
+tcg_gen_or_i64(result2, result2, result1);
+set_avr64(VT, result2, true);
+
+mask = 0x8040201008040201ULL;
+get_avr64(avr, VB, false);
+tcg_gen_movi_i64(result1, 0x0ULL);
+tcg_gen_mov_i64(shifted, avr);
+for (i = 0; i < 8; i++) {
+tcg_gen_andi_i64(tmp, shifted, mask);
+tcg_gen_or_i64(result1, result1, tmp);
+
+tcg_gen_shri_i64(shifted, shifted, 7);
+mask = mask >> 8;
+}
+tcg_gen_andi_i64(result1, result1, mask1);
+
+mask = 0x8040201008040201ULL;
+tcg_gen_movi_i64(result2, 0x0ULL);
+for (i = 0; i < 8; i++) {
+tcg_gen_andi_i64(tmp, avr, mask);
+tcg_gen_or_i64(result2, result2, tmp);
+
+tcg_gen_shli_i64(avr, avr, 7);
+mask = mask <<
[Qemu-devel] [PATCH 6/8] target/ppc: Optimize emulation of vclzw instruction
Optimize Altivec instruction vclzw (Vector Count Leading Zeros Word).
This instruction counts the number of leading zeros of each word element
in source register and places result in the appropriate word element of
destination register.
We perform counting in two iterations of for loop(one for each
doubleword element of source register vB). First thing we do in
loop is placing appropriate doubleword element of vB in variable
avr. Then we perform counting using tcg-s count leading zeros
function. Since it counts leading zeros on 64 bit lenght, we have to
move ith word element to highest 32 bits of variable tmp, or it with
mask(so we get all ones in lowest 32 bits), then perform
tcg_gen_clzi_i64 and move it's result in appropriate word element of
variable result. In the end of each loop iteration we save variable
result to appropriate doubleword element of destination register vD.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 57 -
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 1c34908..7689739 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -878,6 +878,61 @@ static void trans_vgbbd(DisasContext *ctx)
}
/*
+ * vclzw VRT,VRB - Vector Count Leading Zeros Word
+ *
+ * Counting the number of leading zero bits of each word element in source
+ * register and placing result in appropriate word element of destination
+ * register.
+ */
+static void trans_vclzw(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avr = tcg_temp_new_i64();
+TCGv_i64 result = tcg_temp_new_i64();
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 mask = tcg_const_i64(0xULL);
+int i;
+
+for (i = 0; i < 2; i++) {
+if (i == 0) {
+/* Get high doubleword element of vB in avr. */
+get_avr64(avr, VB, true);
+} else {
+/* Get low doubleword element of vB in avr. */
+get_avr64(avr, VB, false);
+}
+/*
+ * Perform count for every word element using tcg_gen_clzi_i64.
+ * Since it counts leading zeros on 64 bit lenght, we have to move
+ * ith word element to highest 32 bits of tmp, or it with mask(so we
get
+ * all ones in lowest 32 bits), then perform tcg_gen_clzi_i64 and move
+ * it's result in appropriate word element of result.
+ */
+tcg_gen_shli_i64(tmp, avr, 32);
+tcg_gen_or_i64(tmp, tmp, mask);
+tcg_gen_clzi_i64(result, tmp, 64);
+
+tcg_gen_or_i64(tmp, avr, mask);
+tcg_gen_clzi_i64(tmp, tmp, 64);
+tcg_gen_deposit_i64(result, result, tmp, 32, 32);
+
+if (i == 0) {
+/* Place result in high doubleword element of vD. */
+set_avr64(VT, result, true);
+} else {
+/* Place result in low doubleword element of vD. */
+set_avr64(VT, result, false);
+}
+}
+
+tcg_temp_free_i64(avr);
+tcg_temp_free_i64(result);
+tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(mask);
+}
+
+/*
* vclzd VRT,VRB - Vector Count Leading Zeros Doubleword
*
* Counting the number of leading zero bits of each doubleword element in
source
@@ -1413,7 +1468,7 @@ GEN_VAFORM_PAIRED(vmaddfp, vnmsubfp, 23)
GEN_VXFORM_NOA(vclzb, 1, 28)
GEN_VXFORM_NOA(vclzh, 1, 29)
-GEN_VXFORM_NOA(vclzw, 1, 30)
+GEN_VXFORM_TRANS(vclzw, 1, 30)
GEN_VXFORM_TRANS(vclzd, 1, 31)
GEN_VXFORM_NOA_2(vnegw, 1, 24, 6)
GEN_VXFORM_NOA_2(vnegd, 1, 24, 7)
--
2.7.4
[Qemu-devel] [PATCH 5/8] target/ppc: Optimize emulation of vclzd instruction
Optimize Altivec instruction vclzd (Vector Count Leading Zeros Doubleword).
This instruction counts the number of leading zeros of each doubleword element
in source register and places result in the appropriate doubleword element of
destination register.
Using tcg-s count leading zeros instruction two times(once for each
doubleword element of source register vB) and placing result in
appropriate doubleword element of destination register vD.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 010f337..1c34908 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -877,6 +877,32 @@ static void trans_vgbbd(DisasContext *ctx)
tcg_temp_free_i64(result2);
}
+/*
+ * vclzd VRT,VRB - Vector Count Leading Zeros Doubleword
+ *
+ * Counting the number of leading zero bits of each doubleword element in
source
+ * register and placing result in appropriate doubleword element of destination
+ * register.
+ */
+static void trans_vclzd(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 avr = tcg_temp_new_i64();
+
+/* high doubleword */
+get_avr64(avr, VB, true);
+tcg_gen_clzi_i64(avr, avr, 64);
+set_avr64(VT, avr, true);
+
+/* low doubleword */
+get_avr64(avr, VB, false);
+tcg_gen_clzi_i64(avr, avr, 64);
+set_avr64(VT, avr, false);
+
+tcg_temp_free_i64(avr);
+}
+
GEN_VXFORM(vmuloub, 4, 0);
GEN_VXFORM(vmulouh, 4, 1);
GEN_VXFORM(vmulouw, 4, 2);
@@ -1388,7 +1414,7 @@ GEN_VAFORM_PAIRED(vmaddfp, vnmsubfp, 23)
GEN_VXFORM_NOA(vclzb, 1, 28)
GEN_VXFORM_NOA(vclzh, 1, 29)
GEN_VXFORM_NOA(vclzw, 1, 30)
-GEN_VXFORM_NOA(vclzd, 1, 31)
+GEN_VXFORM_TRANS(vclzd, 1, 31)
GEN_VXFORM_NOA_2(vnegw, 1, 24, 6)
GEN_VXFORM_NOA_2(vnegd, 1, 24, 7)
GEN_VXFORM_NOA_2(vextsb2w, 1, 24, 16)
--
2.7.4
[Qemu-devel] [PATCH 8/8] target/ppc: Refactor emulation of vmrgew and vmrgow instructions
Since I found this two instructions implemented with tcg, I refactored
them so they are consistent with other similar implementations that
I introduced in this patch.
Also had to add new dual macro GEN_VXFORM_TRANS_DUAL. We use this macro
if one instruction is realized with direct translation, and second one
with helper.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 62 -
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 8535a31..46c6f34 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -350,6 +350,24 @@ static void glue(gen_, name0##_##name1)(DisasContext *ctx)
\
} \
}
+/*
+ * We use this macro if one instruction is realized with direct
+ * translation, and second one with helper.
+ */
+#define GEN_VXFORM_TRANS_DUAL(name0, flg0, flg2_0, name1, flg1, flg2_1)\
+static void glue(gen_, name0##_##name1)(DisasContext *ctx) \
+{ \
+if ((Rc(ctx->opcode) == 0) && \
+((ctx->insns_flags & flg0) || (ctx->insns_flags2 & flg2_0))) { \
+trans_##name0(ctx);\
+} else if ((Rc(ctx->opcode) == 1) && \
+((ctx->insns_flags & flg1) || (ctx->insns_flags2 & flg2_1))) { \
+gen_##name1(ctx); \
+} else { \
+gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);\
+} \
+}
+
/* Adds support to provide invalid mask */
#define GEN_VXFORM_DUAL_EXT(name0, flg0, flg2_0, inval0,\
name1, flg1, flg2_1, inval1)\
@@ -431,20 +449,13 @@ GEN_VXFORM(vmrglb, 6, 4);
GEN_VXFORM(vmrglh, 6, 5);
GEN_VXFORM(vmrglw, 6, 6);
-static void gen_vmrgew(DisasContext *ctx)
+static void trans_vmrgew(DisasContext *ctx)
{
-TCGv_i64 tmp;
-TCGv_i64 avr;
-int VT, VA, VB;
-if (unlikely(!ctx->altivec_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VPU);
-return;
-}
-VT = rD(ctx->opcode);
-VA = rA(ctx->opcode);
-VB = rB(ctx->opcode);
-tmp = tcg_temp_new_i64();
-avr = tcg_temp_new_i64();
+int VT = rD(ctx->opcode);
+int VA = rA(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 avr = tcg_temp_new_i64();
get_avr64(avr, VB, true);
tcg_gen_shri_i64(tmp, avr, 32);
@@ -462,21 +473,14 @@ static void gen_vmrgew(DisasContext *ctx)
tcg_temp_free_i64(avr);
}
-static void gen_vmrgow(DisasContext *ctx)
+static void trans_vmrgow(DisasContext *ctx)
{
-TCGv_i64 t0, t1;
-TCGv_i64 avr;
-int VT, VA, VB;
-if (unlikely(!ctx->altivec_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VPU);
-return;
-}
-VT = rD(ctx->opcode);
-VA = rA(ctx->opcode);
-VB = rB(ctx->opcode);
-t0 = tcg_temp_new_i64();
-t1 = tcg_temp_new_i64();
-avr = tcg_temp_new_i64();
+int VT = rD(ctx->opcode);
+int VA = rA(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 t1 = tcg_temp_new_i64();
+TCGv_i64 avr = tcg_temp_new_i64();
get_avr64(t0, VB, true);
get_avr64(t1, VA, true);
@@ -1213,14 +1217,14 @@ GEN_VXFORM_ENV(vminfp, 5, 17);
GEN_VXFORM_HETRO(vextublx, 6, 24)
GEN_VXFORM_HETRO(vextuhlx, 6, 25)
GEN_VXFORM_HETRO(vextuwlx, 6, 26)
-GEN_VXFORM_DUAL(vmrgow, PPC_NONE, PPC2_ALTIVEC_207,
+GEN_VXFORM_TRANS_DUAL(vmrgow, PPC_NONE, PPC2_ALTIVEC_207,
vextuwlx, PPC_NONE, PPC2_ISA300)
GEN_VXFORM_HETRO(vextubrx, 6, 28)
GEN_VXFORM_HETRO(vextuhrx, 6, 29)
GEN_VXFORM_HETRO(vextuwrx, 6, 30)
GEN_VXFORM_TRANS(lvsl, 6, 31)
GEN_VXFORM_TRANS(lvsr, 6, 32)
-GEN_VXFORM_DUAL(vmrgew, PPC_NONE, PPC2_ALTIVEC_207, \
+GEN_VXFORM_TRANS_DUAL(vmrgew, PPC_NONE, PPC2_ALTIVEC_207,
vextuwrx, PPC_NONE, PPC2_ISA300)
#define GEN_VXRFORM1(opname, name, str, opc2, opc3) \
--
2.7.4
[Qemu-devel] [PATCH 3/8] target/ppc: Optimize emulation of vpkpx instruction
Optimize altivec instruction vpkpx (Vector Pack Pixel).
Rearranges 8 pixels coded in 6-5-5 pattern (4 from each source register)
into contigous array of bits in the destination register.
In each iteration of outer loop we do the 6-5-5 pack for 2 pixels
of each doubleword element of each source register. The first thing
we do in outer loop is choosing which doubleword element of which
register are we using in current iteration and we place it in avr
variable. Then we perform 6-5-5 pack of pixels on avr variable
in inner for loop(2 iterations, 1 for each pixel) and save result
in tmp variable. In the end of outer for loop, we merge result in
variable called result and save it in appropriate doubleword element
of vD if whole doubleword is finished(every second iteration). Outer
loop has 4 iterations.
Signed-off-by: Stefan Brankovic
---
target/ppc/translate/vmx-impl.inc.c | 93 -
1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 6bd072a..87f69dc 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -593,6 +593,97 @@ static void trans_lvsr(DisasContext *ctx)
}
/*
+ * vpkpx VRT,VRA,VRB - Vector Pack Pixel
+ *
+ * Rearranges 8 pixels coded in 6-5-5 pattern (4 from each source register)
+ * into contigous array of bits in the destination register.
+ */
+static void trans_vpkpx(DisasContext *ctx)
+{
+int VT = rD(ctx->opcode);
+int VA = rA(ctx->opcode);
+int VB = rB(ctx->opcode);
+TCGv_i64 tmp = tcg_temp_new_i64();
+TCGv_i64 shifted = tcg_temp_new_i64();
+TCGv_i64 avr = tcg_temp_new_i64();
+TCGv_i64 result = tcg_temp_new_i64();
+int64_t mask1 = 0x1fULL;
+int64_t mask2 = 0x1fULL << 5;
+int64_t mask3 = 0x3fULL << 10;
+int i, j;
+/*
+ * In each iteration do the 6-5-5 pack for 2 pixels of each doubleword
+ * element of each source register.
+ */
+for (i = 0; i < 4; i++) {
+switch (i) {
+case 0:
+/*
+ * Get high doubleword of vA to perfrom 6-5-5 pack of pixels
+ * 1 and 2.
+ */
+get_avr64(avr, VA, true);
+tcg_gen_movi_i64(result, 0x0ULL);
+break;
+case 1:
+/*
+ * Get low doubleword of vA to perfrom 6-5-5 pack of pixels
+ * 3 and 4.
+ */
+get_avr64(avr, VA, false);
+break;
+case 2:
+/*
+ * Get high doubleword of vB to perfrom 6-5-5 pack of pixels
+ * 5 and 6.
+ */
+get_avr64(avr, VB, true);
+tcg_gen_movi_i64(result, 0x0ULL);
+break;
+case 3:
+/*
+ * Get low doubleword of vB to perfrom 6-5-5 pack of pixels
+ * 7 and 8.
+ */
+get_avr64(avr, VB, false);
+break;
+}
+/* Perform the packing for 2 pixels(each iteration for 1). */
+tcg_gen_movi_i64(tmp, 0x0ULL);
+for (j = 0; j < 2; j++) {
+tcg_gen_shri_i64(shifted, avr, (j * 16 + 3));
+tcg_gen_andi_i64(shifted, shifted, mask1 << (j * 16));
+tcg_gen_or_i64(tmp, tmp, shifted);
+
+tcg_gen_shri_i64(shifted, avr, (j * 16 + 6));
+tcg_gen_andi_i64(shifted, shifted, mask2 << (j * 16));
+tcg_gen_or_i64(tmp, tmp, shifted);
+
+tcg_gen_shri_i64(shifted, avr, (j * 16 + 9));
+tcg_gen_andi_i64(shifted, shifted, mask3 << (j * 16));
+tcg_gen_or_i64(tmp, tmp, shifted);
+}
+if ((i == 0) || (i == 2)) {
+tcg_gen_shli_i64(tmp, tmp, 32);
+}
+tcg_gen_or_i64(result, result, tmp);
+if (i == 1) {
+/* Place packed pixels 1:4 to high doubleword of vD. */
+set_avr64(VT, result, true);
+}
+if (i == 3) {
+/* Place packed pixels 5:8 to low doubleword of vD. */
+set_avr64(VT, result, false);
+}
+}
+
+tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(shifted);
+tcg_temp_free_i64(avr);
+tcg_temp_free_i64(result);
+}
+
+/*
* vsl VRT,VRA,VRB - Vector Shift Left
*
* Shifting left 128 bit value of vA by value specified in bits 125-127 of vB.
@@ -813,7 +904,7 @@ GEN_VXFORM_ENV(vpksdus, 7, 21);
GEN_VXFORM_ENV(vpkshss, 7, 6);
GEN_VXFORM_ENV(vpkswss, 7, 7);
GEN_VXFORM_ENV(vpksdss, 7, 23);
-GEN_VXFORM(vpkpx, 7, 12);
+GEN_VXFORM_TRANS(vpkpx, 7, 12);
GEN_VXFORM_ENV(vsum4ubs, 4, 24);
GEN_VXFORM_ENV(vsum4sbs, 4, 28);
GEN_VXFORM_ENV(vsum4shs, 4, 25);
--
2.7.4
Re: [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test
On Wed, Jun 05, 2019 at 09:57:06PM +0100, Richard Henderson wrote:
> This will build with older toolchains, without the upstream support
> for -mbranch-protection. Such a toolchain will produce a warning
> in such cases,
>
> ld: warning: /tmp/ccyZt0kq.o: unsupported GNU_PROPERTY_TYPE (5) \
> type: 0xc000
>
> but the still places the note at the correct location in the binary
> for processing by the runtime loader.
>
> Signed-off-by: Richard Henderson
> ---
> tests/tcg/aarch64/bti-1.c | 77 +++
> tests/tcg/aarch64/bti-crt.inc.c | 69 +++
> tests/tcg/aarch64/Makefile.target | 3 ++
> 3 files changed, 149 insertions(+)
> create mode 100644 tests/tcg/aarch64/bti-1.c
> create mode 100644 tests/tcg/aarch64/bti-crt.inc.c
>
> diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c
> new file mode 100644
> index 00..2aee57ea7a
> --- /dev/null
> +++ b/tests/tcg/aarch64/bti-1.c
> @@ -0,0 +1,77 @@
> +/*
> + * Branch target identification, basic notskip cases.
> + */
> +
> +#include "bti-crt.inc.c"
> +
> +/*
> + * Work around lack of -mbranch-protection=standard in older toolchains.
> + * The signal handler is invoked by the kernel with PSTATE.BTYPE=2, which
> + * means that the handler must begin with a marker like BTI_C.
> + */
> +asm("skip2_sigill1:\n\
> + hint#34\n\
> + b skip2_sigill2\n\
> +.type skip2_sigill1,%function\n\
> +.size skip2_sigill1,8");
> +
> +extern void skip2_sigill1(int sig, siginfo_t *info, ucontext_t *uc)
> +__attribute__((visibility("hidden")));
> +
> +static void __attribute__((used))
> +skip2_sigill2(int sig, siginfo_t *info, ucontext_t *uc)
> +{
> +uc->uc_mcontext.pc += 8;
> +uc->uc_mcontext.pstate = 1;
> +}
> +
> +#define NOP "nop"
> +#define BTI_N "hint #32"
> +#define BTI_C "hint #34"
> +#define BTI_J "hint #36"
> +#define BTI_JC"hint #38"
> +
> +#define BTYPE_1(DEST) \
> +asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \
> +: "=r"(skipped) : : "x16")
> +
> +#define BTYPE_2(DEST) \
> +asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \
> +: "=r"(skipped) : : "x16", "x30")
> +
> +#define BTYPE_3(DEST) \
> +asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \
> +: "=r"(skipped) : : "x15")
> +
> +#define TEST(WHICH, DEST, EXPECT) \
> +do { WHICH(DEST); fail += skipped ^ EXPECT; } while (0)
> +
> +
> +int main()
> +{
> +int fail = 0;
> +int skipped;
> +
> +/* Signal-like with SA_SIGINFO. */
> +signal_info(SIGILL, skip2_sigill1);
> +
> +TEST(BTYPE_1, NOP, 1);
> +TEST(BTYPE_1, BTI_N, 1);
> +TEST(BTYPE_1, BTI_C, 0);
> +TEST(BTYPE_1, BTI_J, 0);
> +TEST(BTYPE_1, BTI_JC, 0);
> +
> +TEST(BTYPE_2, NOP, 1);
> +TEST(BTYPE_2, BTI_N, 1);
> +TEST(BTYPE_2, BTI_C, 0);
> +TEST(BTYPE_2, BTI_J, 1);
> +TEST(BTYPE_2, BTI_JC, 0);
> +
> +TEST(BTYPE_3, NOP, 1);
> +TEST(BTYPE_3, BTI_N, 1);
> +TEST(BTYPE_3, BTI_C, 1);
> +TEST(BTYPE_3, BTI_J, 0);
> +TEST(BTYPE_3, BTI_JC, 0);
> +
> +return fail;
> +}
> diff --git a/tests/tcg/aarch64/bti-crt.inc.c b/tests/tcg/aarch64/bti-crt.inc.c
> new file mode 100644
> index 00..bb363853de
> --- /dev/null
> +++ b/tests/tcg/aarch64/bti-crt.inc.c
> @@ -0,0 +1,69 @@
> +/*
> + * Minimal user-environment for testing BTI.
> + *
> + * Normal libc is not built with BTI support enabled, and so could
> + * generate a BTI TRAP before ever reaching main.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +int main(void);
> +
> +void _start(void)
> +{
> +exit(main());
> +}
> +
> +void exit(int ret)
> +{
> +register int x0 __asm__("x0") = ret;
> +register int x8 __asm__("x8") = __NR_exit;
> +
> +asm volatile("svc #0" : : "r"(x0), "r"(x8));
> +__builtin_unreachable();
> +}
> +
> +/*
> + * Irritatingly, the user API struct sigaction does not match the
> + * kernel API struct sigaction. So for simplicity, isolate the
> + * kernel ABI here, and make this act like signal.
> + */
> +void signal_info(int sig, void (*fn)(int, siginfo_t *, ucontext_t *))
> +{
> +struct kernel_sigaction {
> +void (*handler)(int, siginfo_t *, ucontext_t *);
> +unsigned long flags;
> +unsigned long restorer;
> +unsigned long mask;
> +} sa = { fn, SA_SIGINFO, 0, 0 };
> +
> +register int x0 __asm__("x0") = sig;
> +register void *x1 __asm__("x1") = &sa;
> +register void *x2 __asm__("x2") = 0;
> +register int x3 __asm__("x3") = sizeof(unsigned long);
> +register int x8 __asm__("x8") = __NR_rt_sigaction;
> +
> +asm volatile("svc #0"
> + : : "r"(x0), "r"(x1), "r"(x2), "r"(x3), "r"(x8) : "memory");
> +}
> +
> +/*
> + * Create the PT_NOTE that will enable BTI in the page tables.
> + * This will be created by the compiler with -mbranch-protection=standard,
> + * but as of 2019-03-29, this is has not been committe
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
* Liran Alon (liran.a...@oracle.com) wrote: > > > > On 6 Jun 2019, at 12:23, Dr. David Alan Gilbert wrote: > > > > * Liran Alon (liran.a...@oracle.com) wrote: > >> > >> > >>> On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert > >>> wrote: > >>> > >>> * Liran Alon (liran.a...@oracle.com) wrote: > Hi, > > Looking at QEMU source code, I am puzzled regarding how migration > backwards compatibility is preserved regarding X86CPU. > > As I understand it, fields that are based on KVM capabilities and guest > runtime usage are defined in VMState subsections in order to not send > them if not necessary. > This is done such that in case they are not needed and we migrate to an > old QEMU which don’t support loading this state, migration will still > succeed > (As .needed() method will return false and therefore this state won’t be > sent as part of migration stream). > Furthermore, in case .needed() returns true and old QEMU don’t support > loading this state, migration fails. As it should because we are aware > that guest state > is not going to be restored properly on destination. > > I’m puzzled about what will happen in the following scenario: > 1) Source is running new QEMU with new KVM that supports save of some > VMState subsection. > 2) Destination is running new QEMU that supports load this state but > with old kernel that doesn’t know how to load this state. > > I would have expected in this case that if source .needed() returns > true, then migration will fail because of lack of support in destination > kernel. > However, it seems from current QEMU code that this will actually succeed > in many cases. > > For example, if msr_smi_count is sent as part of migration stream (See > vmstate_msr_smi_count) and destination have has_msr_smi_count==false, > then destination will succeed loading migration stream but > kvm_put_msrs() will actually ignore env->msr_smi_count and will > successfully load guest state. > Therefore, migration will succeed even though it should have failed… > > It seems to me that QEMU should have for every such VMState subsection, > a .post_load() method that verifies that relevant capability is > supported by kernel > and otherwise fail migration. > > What do you think? Should I really create a patch to modify all these > CPUX86 VMState subsections to behave like this? > >>> > >>> I don't know the x86 specific side that much; but from my migration side > >>> the answer should mostly be through machine types - indeed for smi-count > >>> there's a property 'x-migrate-smi-count' which is off for machine types > >>> pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old > >>> kernel you should stick to the old machine types. > >>> > >>> There's nothing guarding running the new machine type on old-kernels; > >>> and arguably we should have a check at startup that complains if > >>> your kernel is missing something the machine type uses. > >>> However, that would mean that people running with -M pc would fail > >>> on old kernels. > >>> > >>> A post-load is also a valid check; but one question is whether, > >>> for a particular register, the pain is worth it - it depends on the > >>> symptom that the missing state causes. If it's minor then you might > >>> conclude it's not worth a failed migration; if it's a hung or > >>> corrupt guest then yes it is. Certainly a warning printed is worth > >>> it. > >>> > >>> Dave > >> > >> I think we should have flags that allow user to specify which VMState > >> subsections user explicitly allow to avoid restore even though they are > >> required to fully restore guest state. > >> But it seems to me that the behaviour should be to always fail migration > >> in case we load a VMState subsections that we are unable to restore unless > >> user explicitly specified this is ok > >> for this specific subsection. > >> Therefore, it seems that for every VMState subsection that it’s restore is > >> based on kernel capability we should: > >> 1) Have a user-controllable flag (which is also tied to machine-type?) to > >> explicitly allow avoid restoring this state if cannot. Default should be > >> “false”. > >> 2) Have a .post_load() method that verifies we have required kernel > >> capability to restore this state, unless flag (1) was specified as “true”. > > > > This seems a lot of flags; users aren't going to know what to do with > > all of them; I don't see what will set/control them. > > True but I think users will want to specify only for a handful of VMState > subsections that it is OK to not restore them even thought hey are deemed > needed by source QEMU. > We can create flags only for those VMState subsections. > User should set these flags explicitly on QEMU command-line. As a
Re: [Qemu-devel] [PULL 0/5] Migration patches
On Wed, 5 Jun 2019 at 12:55, Juan Quintela wrote: > > The following changes since commit 47fbad45d47af8af784bb12a5719489edcd89b4c: > > Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging > (2019-06-04 17:22:42 +0100) > > are available in the Git repository at: > > https://github.com/juanquintela/qemu.git tags/migration-pull-request > > for you to fetch changes up to 03158519384f15890d587937bd1b3ea699898e55: > > migratioin/ram: leave RAMBlock->bmap blank on allocating (2019-06-05 > 12:44:03 +0200) > > > Migration Pull request > > - Fd fixes and test (yuri) > - several fixes (wei) > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1 for any user-visible changes. -- PMM
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
> On 6 Jun 2019, at 13:39, Dr. David Alan Gilbert wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> >> >>> On 6 Jun 2019, at 12:23, Dr. David Alan Gilbert wrote: >>> >>> * Liran Alon (liran.a...@oracle.com) wrote: > On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert > wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> Hi, >> >> Looking at QEMU source code, I am puzzled regarding how migration >> backwards compatibility is preserved regarding X86CPU. >> >> As I understand it, fields that are based on KVM capabilities and guest >> runtime usage are defined in VMState subsections in order to not send >> them if not necessary. >> This is done such that in case they are not needed and we migrate to an >> old QEMU which don’t support loading this state, migration will still >> succeed >> (As .needed() method will return false and therefore this state won’t be >> sent as part of migration stream). >> Furthermore, in case .needed() returns true and old QEMU don’t support >> loading this state, migration fails. As it should because we are aware >> that guest state >> is not going to be restored properly on destination. >> >> I’m puzzled about what will happen in the following scenario: >> 1) Source is running new QEMU with new KVM that supports save of some >> VMState subsection. >> 2) Destination is running new QEMU that supports load this state but >> with old kernel that doesn’t know how to load this state. >> >> I would have expected in this case that if source .needed() returns >> true, then migration will fail because of lack of support in destination >> kernel. >> However, it seems from current QEMU code that this will actually succeed >> in many cases. >> >> For example, if msr_smi_count is sent as part of migration stream (See >> vmstate_msr_smi_count) and destination have has_msr_smi_count==false, >> then destination will succeed loading migration stream but >> kvm_put_msrs() will actually ignore env->msr_smi_count and will >> successfully load guest state. >> Therefore, migration will succeed even though it should have failed… >> >> It seems to me that QEMU should have for every such VMState subsection, >> a .post_load() method that verifies that relevant capability is >> supported by kernel >> and otherwise fail migration. >> >> What do you think? Should I really create a patch to modify all these >> CPUX86 VMState subsections to behave like this? > > I don't know the x86 specific side that much; but from my migration side > the answer should mostly be through machine types - indeed for smi-count > there's a property 'x-migrate-smi-count' which is off for machine types > pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old > kernel you should stick to the old machine types. > > There's nothing guarding running the new machine type on old-kernels; > and arguably we should have a check at startup that complains if > your kernel is missing something the machine type uses. > However, that would mean that people running with -M pc would fail > on old kernels. > > A post-load is also a valid check; but one question is whether, > for a particular register, the pain is worth it - it depends on the > symptom that the missing state causes. If it's minor then you might > conclude it's not worth a failed migration; if it's a hung or > corrupt guest then yes it is. Certainly a warning printed is worth > it. > > Dave I think we should have flags that allow user to specify which VMState subsections user explicitly allow to avoid restore even though they are required to fully restore guest state. But it seems to me that the behaviour should be to always fail migration in case we load a VMState subsections that we are unable to restore unless user explicitly specified this is ok for this specific subsection. Therefore, it seems that for every VMState subsection that it’s restore is based on kernel capability we should: 1) Have a user-controllable flag (which is also tied to machine-type?) to explicitly allow avoid restoring this state if cannot. Default should be “false”. 2) Have a .post_load() method that verifies we have required kernel capability to restore this state, unless flag (1) was specified as “true”. >>> >>> This seems a lot of flags; users aren't going to know what to do with >>> all of them; I don't see what will set/control them. >> >> True but I think users will want to specify only for a handful of VMState >> subsections that it is OK to not restore them even thought hey are deemed >> needed by source QEMU. >> We can create flags only for those VMState subsect
[Qemu-devel] [PULL 07/16] hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly
From: Philippe Mathieu-Daudé
Since the BusState is accesible from the SCSIBus object,
it is pointless to use qbus_reset_all_fn.
Use qbus_reset_all() directly.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Dmitry Fleytman
Message-Id: <20190528164020.32250-2-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/scsi/vmw_pvscsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 584b4be07e79..c39e33fa3599 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -440,7 +440,7 @@ static void
pvscsi_reset_adapter(PVSCSIState *s)
{
s->resetting++;
-qbus_reset_all_fn(&s->bus);
+qbus_reset_all(BUS(&s->bus));
s->resetting--;
pvscsi_process_completion_queue(s);
assert(QTAILQ_EMPTY(&s->pending_queue));
@@ -848,7 +848,7 @@ pvscsi_on_cmd_reset_bus(PVSCSIState *s)
trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_RESET_BUS");
s->resetting++;
-qbus_reset_all_fn(&s->bus);
+qbus_reset_all(BUS(&s->bus));
s->resetting--;
return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
}
--
2.21.0
[Qemu-devel] [PULL 11/16] hw/usb-storage: Use the QOM DEVICE() macro to access DeviceState.qdev
From: Philippe Mathieu-Daudé Rather than looking inside the definition of a DeviceState with "s->qdev", use the QOM prefered style: "DEVICE(s)". This patch was generated using the following Coccinelle script: // Use DEVICE() macros to access DeviceState.qdev @use_device_macro_to_access_qdev@ expression obj; identifier dev; @@ -&obj->dev.qdev +DEVICE(obj) Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Gerd Hoffmann Message-Id: <20190528164020.32250-9-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/usb/dev-storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index cd5551d94f35..0e4e93ef1673 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -616,7 +616,7 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp) * The hack is probably a bad idea. */ blk_ref(blk); -blk_detach_dev(blk, &s->dev.qdev); +blk_detach_dev(blk, DEVICE(s)); s->conf.blk = NULL; usb_desc_create_serial(dev); -- 2.21.0
[Qemu-devel] [PULL 06/16] docs/devel/build-system: Update an example
From: Philippe Mathieu-Daudé The default-configs/ example added in 717171bd2025 is no more accurate since fa212a2b8b60 (and various further other commits). The Kconfig build system is now in place. Use the aarch64-softmmu config as example. Reviewed-by: Thomas Huth Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20190529140504.21580-1-phi...@redhat.com> Signed-off-by: Laurent Vivier --- docs/devel/build-system.txt | 19 +++ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt index addd274eebd9..41bd08ea3ab3 100644 --- a/docs/devel/build-system.txt +++ b/docs/devel/build-system.txt @@ -413,18 +413,13 @@ context. - default-configs/*.mak The files under default-configs/ control what emulated hardware is built -into each QEMU system and userspace emulator targets. They merely -contain a long list of config variable definitions. For example, -default-configs/x86_64-softmmu.mak has: - - include sound.mak - include usb.mak - CONFIG_QXL=$(CONFIG_SPICE) - CONFIG_VGA_ISA=y - CONFIG_VGA_CIRRUS=y - CONFIG_VMWARE_VGA=y - CONFIG_VIRTIO_VGA=y - ...snip... +into each QEMU system and userspace emulator targets. They merely contain +a list of config variable definitions like the machines that should be +included. For example, default-configs/aarch64-softmmu.mak has: + + include arm-softmmu.mak + CONFIG_XLNX_ZYNQMP_ARM=y + CONFIG_XLNX_VERSAL=y These files rarely need changing unless new devices / hardware need to be enabled for a particular system/userspace emulation target -- 2.21.0
[Qemu-devel] [PULL 04/16] util: Adjust qemu_guest_getrandom_nofail for Coverity
From: Richard Henderson
Explicitly ignore the return value of qemu_guest_getrandom.
Because we use error_fatal, all errors are already caught.
Fixes: CID 1401701
Signed-off-by: Richard Henderson
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20190530173824.30699-1-richard.hender...@linaro.org>
Signed-off-by: Laurent Vivier
---
util/guest-random.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/guest-random.c b/util/guest-random.c
index e8124a3cad48..00a08fd9819c 100644
--- a/util/guest-random.c
+++ b/util/guest-random.c
@@ -56,7 +56,7 @@ int qemu_guest_getrandom(void *buf, size_t len, Error **errp)
void qemu_guest_getrandom_nofail(void *buf, size_t len)
{
-qemu_guest_getrandom(buf, len, &error_fatal);
+(void)qemu_guest_getrandom(buf, len, &error_fatal);
}
uint64_t qemu_guest_random_seed_thread_part1(void)
--
2.21.0
[Qemu-devel] [PULL 09/16] hw/s390x/event-facility: Use the QOM BUS() macro to access BusState.qbus
From: Philippe Mathieu-Daudé Rather than looking inside the definition of a BusState with "s->bus.qbus", use the QOM prefered style: "BUS(&s->bus)". This patch was generated using the following Coccinelle script: // Use BUS() macros to access BusState.qbus @use_bus_macro_to_access_qbus@ expression obj; identifier bus; @@ -&obj->bus.qbus +BUS(&obj->bus) Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Cornelia Huck Message-Id: <20190528164020.32250-5-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/s390x/event-facility.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c index ee5b83448b11..e574223a226b 100644 --- a/hw/s390x/event-facility.c +++ b/hw/s390x/event-facility.c @@ -466,12 +466,12 @@ static void init_event_facility(Object *obj) new = object_new(TYPE_SCLP_QUIESCE); object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL); object_unref(new); -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); new = object_new(TYPE_SCLP_CPU_HOTPLUG); object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL); object_unref(new); -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); /* the facility will automatically realize the devices via the bus */ } -- 2.21.0
[Qemu-devel] [PULL 03/16] vhost: fix incorrect print type
From: Jie Wang
fix incorrect print type in vhost_virtqueue_stop
Signed-off-by: Jie Wang
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <1556605773-42019-1-git-send-email-wangji...@huawei.com>
Signed-off-by: Laurent Vivier
---
hw/virtio/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7f61018f2a3f..286bb27c6539 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1081,7 +1081,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
if (r < 0) {
-VHOST_OPS_DEBUG("vhost VQ %d ring restore failed: %d", idx, r);
+VHOST_OPS_DEBUG("vhost VQ %u ring restore failed: %d", idx, r);
/* Connection to the backend is broken, so let's sync internal
* last avail idx to the device used idx.
*/
--
2.21.0
[Qemu-devel] [PULL 14/16] hw/sd: Use the QOM BUS() macro to access BusState.qbus
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a BusState with "s->bus.qbus",
use the QOM prefered style: "BUS(&s->bus)".
This patch was generated using the following Coccinelle script:
// Use BUS() macros to access BusState.qbus
@use_bus_macro_to_access_qbus@
expression obj;
identifier bus;
@@
-&obj->bus.qbus
+BUS(&obj->bus)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
Message-Id: <20190528164020.32250-6-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/sd/milkymist-memcard.c | 2 +-
hw/sd/ssi-sd.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index df42aa1c545b..dd1ba649d9c0 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -277,7 +277,7 @@ static void milkymist_memcard_realize(DeviceState *dev,
Error **errp)
/* FIXME use a qdev drive property instead of drive_get_next() */
dinfo = drive_get_next(IF_SD);
blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
-carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk, &err);
object_property_set_bool(OBJECT(carddev), true, "realized", &err);
if (err) {
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 623d0333e863..25e1009277ed 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -249,7 +249,7 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
/* Create and plug in the sd card */
/* FIXME use a qdev drive property instead of drive_get_next() */
dinfo = drive_get_next(IF_SD);
-carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+carddev = qdev_create(BUS(&s->sdbus), TYPE_SD_CARD);
if (dinfo) {
qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo),
&err);
}
--
2.21.0
[Qemu-devel] [PULL 08/16] hw/pci-bridge: Use the QOM BUS() macro to access BusState.qbus
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a BusState with "s->bus.qbus",
use the QOM prefered style: "BUS(&s->bus)".
This patch was generated using the following Coccinelle script:
// Use BUS() macros to access BusState.qbus
@use_bus_macro_to_access_qbus@
expression obj;
identifier bus;
@@
-&obj->bus.qbus
+BUS(&obj->bus)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marcel Apfelbaum
Message-Id: <20190528164020.32250-4-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/pci/pci_bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index c6d9ded3203c..8d954885c0a1 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -273,7 +273,7 @@ void pci_bridge_write_config(PCIDevice *d,
newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
/* Trigger hot reset on 0->1 transition. */
-qbus_reset_all(&s->sec_bus.qbus);
+qbus_reset_all(BUS(&s->sec_bus));
}
}
--
2.21.0
[Qemu-devel] [PULL 01/16] hw/rdma: Delete unused headers inclusion
From: Yuval Shaia This is a trivial cleanup patch. Signed-off-by: Yuval Shaia Reviewed-by: Kamal Heib Message-Id: <20190505105112.22691-1-yuval.sh...@oracle.com> Signed-off-by: Laurent Vivier --- hw/rdma/rdma_backend.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c index cf34874e9d2f..c39051068d1e 100644 --- a/hw/rdma/rdma_backend.c +++ b/hw/rdma/rdma_backend.c @@ -14,16 +14,9 @@ */ #include "qemu/osdep.h" -#include "sysemu/sysemu.h" -#include "qapi/error.h" -#include "qapi/qmp/qlist.h" -#include "qapi/qmp/qnum.h" #include "qapi/qapi-events-rdma.h" #include -#include -#include -#include #include "contrib/rdmacm-mux/rdmacm-mux.h" #include "trace.h" -- 2.21.0
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
* Liran Alon (liran.a...@oracle.com) wrote: > > > > On 6 Jun 2019, at 13:39, Dr. David Alan Gilbert wrote: > > > > * Liran Alon (liran.a...@oracle.com) wrote: > >> > >> > >>> On 6 Jun 2019, at 12:23, Dr. David Alan Gilbert > >>> wrote: > >>> > >>> * Liran Alon (liran.a...@oracle.com) wrote: > > > > On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert > > wrote: > > > > * Liran Alon (liran.a...@oracle.com) wrote: > >> Hi, > >> > >> Looking at QEMU source code, I am puzzled regarding how migration > >> backwards compatibility is preserved regarding X86CPU. > >> > >> As I understand it, fields that are based on KVM capabilities and > >> guest runtime usage are defined in VMState subsections in order to not > >> send them if not necessary. > >> This is done such that in case they are not needed and we migrate to > >> an old QEMU which don’t support loading this state, migration will > >> still succeed > >> (As .needed() method will return false and therefore this state won’t > >> be sent as part of migration stream). > >> Furthermore, in case .needed() returns true and old QEMU don’t support > >> loading this state, migration fails. As it should because we are aware > >> that guest state > >> is not going to be restored properly on destination. > >> > >> I’m puzzled about what will happen in the following scenario: > >> 1) Source is running new QEMU with new KVM that supports save of some > >> VMState subsection. > >> 2) Destination is running new QEMU that supports load this state but > >> with old kernel that doesn’t know how to load this state. > >> > >> I would have expected in this case that if source .needed() returns > >> true, then migration will fail because of lack of support in > >> destination kernel. > >> However, it seems from current QEMU code that this will actually > >> succeed in many cases. > >> > >> For example, if msr_smi_count is sent as part of migration stream (See > >> vmstate_msr_smi_count) and destination have has_msr_smi_count==false, > >> then destination will succeed loading migration stream but > >> kvm_put_msrs() will actually ignore env->msr_smi_count and will > >> successfully load guest state. > >> Therefore, migration will succeed even though it should have failed… > >> > >> It seems to me that QEMU should have for every such VMState > >> subsection, a .post_load() method that verifies that relevant > >> capability is supported by kernel > >> and otherwise fail migration. > >> > >> What do you think? Should I really create a patch to modify all these > >> CPUX86 VMState subsections to behave like this? > > > > I don't know the x86 specific side that much; but from my migration side > > the answer should mostly be through machine types - indeed for smi-count > > there's a property 'x-migrate-smi-count' which is off for machine types > > pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old > > kernel you should stick to the old machine types. > > > > There's nothing guarding running the new machine type on old-kernels; > > and arguably we should have a check at startup that complains if > > your kernel is missing something the machine type uses. > > However, that would mean that people running with -M pc would fail > > on old kernels. > > > > A post-load is also a valid check; but one question is whether, > > for a particular register, the pain is worth it - it depends on the > > symptom that the missing state causes. If it's minor then you might > > conclude it's not worth a failed migration; if it's a hung or > > corrupt guest then yes it is. Certainly a warning printed is worth > > it. > > > > Dave > > I think we should have flags that allow user to specify which VMState > subsections user explicitly allow to avoid restore even though they are > required to fully restore guest state. > But it seems to me that the behaviour should be to always fail migration > in case we load a VMState subsections that we are unable to restore > unless user explicitly specified this is ok > for this specific subsection. > Therefore, it seems that for every VMState subsection that it’s restore > is based on kernel capability we should: > 1) Have a user-controllable flag (which is also tied to machine-type?) > to explicitly allow avoid restoring this state if cannot. Default should > be “false”. > 2) Have a .post_load() method that verifies we have required kernel > capability to restore this state, unless flag (1) was specified as > “true”. > >>> > >>> This seems a lot of flags; users aren't going to know what to do with > >>> all of them; I don't see what will s
[Qemu-devel] [PULL 13/16] hw/audio/ac97: Use the QOM DEVICE() macro to access DeviceState.qdev
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a DeviceState with
"s->qdev", use the QOM prefered style: "DEVICE(s)".
This patch was generated using the following Coccinelle script
(with a bit of manual fix-up, removing an extra space to please
checkpatch.pl):
// Use DEVICE() macros to access DeviceState.qdev
@use_device_macro_to_access_qdev@
expression obj;
identifier dev;
@@
-&obj->dev.qdev
+DEVICE(obj)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell .
Message-Id: <20190528164020.32250-7-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/audio/ac97.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 2265622d44a2..a4e8d99e7775 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1388,7 +1388,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
AUD_register_card ("ac97", &s->card);
-ac97_on_reset (&s->dev.qdev);
+ac97_on_reset(DEVICE(s));
}
static void ac97_exit(PCIDevice *dev)
--
2.21.0
Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh
On Wed, Jun 05, 2019 at 11:36:54PM +0200, Pino Toscano wrote: > Rewrite the implementation of the ssh block driver to use libssh instead > of libssh2. The libssh library has various advantages over libssh2: > - easier API for authentication (for example for using ssh-agent) > - easier API for known_hosts handling > - supports newer types of keys in known_hosts > > Use APIs/features available in libssh 0.8 conditionally, to support > older versions (which are not recommended though). > > Signed-off-by: Pino Toscano > --- > > Changes from v5: > - adapt to newer tracing APIs > - disable ssh compression (mimic what libssh2 does by default) > - use build time checks for libssh 0.8, and use newer APIs directly > > Changes from v4: > - fix wrong usages of error_setg/session_error_setg/sftp_error_setg > - fix few return code checks > - remove now-unused parameters in few internal functions > - allow authentication with "none" method > - switch to unsigned int for the port number > - enable TCP_NODELAY on the socket > - fix one reference error message in iotest 207 > > Changes from v3: > - fix socket cleanup in connect_to_ssh() > - add comments about the socket cleanup > - improve the error reporting (closer to what was with libssh2) > - improve EOF detection on sftp_read() > > Changes from v2: > - used again an own fd > - fixed co_yield() implementation > > Changes from v1: > - fixed jumbo packets writing > - fixed missing 'err' assignment > - fixed commit message > > block/Makefile.objs| 6 +- > block/ssh.c| 610 +++-- > block/trace-events | 14 +- > configure | 62 ++-- > tests/qemu-iotests/207.out | 2 +- > 5 files changed, 351 insertions(+), 343 deletions(-) > diff --git a/configure b/configure > index b091b82cb3..bfdd70c40a 100755 > --- a/configure > +++ b/configure > @@ -3914,43 +3914,17 @@ EOF > fi > > ## > -# libssh2 probe > -min_libssh2_version=1.2.8 The commit message says we're conditionally using APIs from 0.8.0, but doesn't say what minimum version we actually need and there's no check here. In terms of our supported build platforms, the oldest libssh I see is RHEL-7 with 0.7.1. So assume it does actually compile on RHEL-7, then it is desirable to have a min_libssh_Version=0.7.1 set here & checked below. At some future date when support platforms changes, we'll then be able to tweak it to 0.8.0 and drop the conditional compilation. > -if test "$libssh2" != "no" ; then > - if $pkg_config --atleast-version=$min_libssh2_version libssh2; then > -libssh2_cflags=$($pkg_config libssh2 --cflags) > -libssh2_libs=$($pkg_config libssh2 --libs) > -libssh2=yes > +# libssh probe > +if test "$libssh" != "no" ; then > + if $pkg_config --exists libssh; then > +libssh_cflags=$($pkg_config libssh --cflags) > +libssh_libs=$($pkg_config libssh --libs) > +libssh=yes >else > -if test "$libssh2" = "yes" ; then > - error_exit "libssh2 >= $min_libssh2_version required for > --enable-libssh2" > +if test "$libssh" = "yes" ; then > + error_exit "libssh required for --enable-libssh" > fi > -libssh2=no > - fi > -fi Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PULL 05/16] test: Fix make target check-report.tap
From: Markus Armbruster Fix a fat-fingered invocation of tap-merge.pl in the recipe of target check-report.tap. Fixes: 9df43317b82 "test: replace gtester with a TAP driver" Signed-off-by: Markus Armbruster Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Message-Id: <20190604080010.23186-1-arm...@redhat.com> Signed-off-by: Laurent Vivier --- tests/Makefile.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 46a36c2c9527..34df040583dc 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -904,7 +904,7 @@ check-report-unit.tap: $(check-unit-y) # Reports and overall runs check-report.tap: $(patsubst %,check-report-qtest-%.tap, $(QTEST_TARGETS)) check-report-unit.tap - $(call quiet-command,./scripts/tap-merge.py $^ > $@,"GEN","$@") + $(call quiet-command, cat $^ | scripts/tap-merge.pl >$@,"GEN","$@") # FPU Emulation tests (aka softfloat) # -- 2.21.0
[Qemu-devel] [PULL 12/16] hw/vfio/pci: Use the QOM DEVICE() macro to access DeviceState.qdev
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a DeviceState with
"s->qdev", use the QOM prefered style: "DEVICE(s)".
This patch was generated using the following Coccinelle script:
// Use DEVICE() macros to access DeviceState.qdev
@use_device_macro_to_access_qdev@
expression obj;
identifier dev;
@@
-&obj->dev.qdev
+DEVICE(obj)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Acked-by: Alex Williamson
Message-Id: <20190528164020.32250-10-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/vfio/pci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8e555db12ec0..2a4091d216e2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2717,7 +2717,7 @@ static void vfio_req_notifier_handler(void *opaque)
return;
}
-qdev_unplug(&vdev->pdev.qdev, &err);
+qdev_unplug(DEVICE(vdev), &err);
if (err) {
warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
@@ -2839,7 +2839,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
vdev->vbasedev.ops = &vfio_pci_ops;
vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
-vdev->vbasedev.dev = &vdev->pdev.qdev;
+vdev->vbasedev.dev = DEVICE(vdev);
tmp = g_strdup_printf("%s/iommu_group", vdev->vbasedev.sysfsdev);
len = readlink(tmp, group_path, sizeof(group_path));
--
2.21.0
[Qemu-devel] [PULL 00/16] Trivial branch patches
The following changes since commit 47fbad45d47af8af784bb12a5719489edcd89b4c: Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-06-04 17:22:42 +0100) are available in the Git repository at: git://github.com/vivier/qemu.git tags/trivial-branch-pull-request for you to fetch changes up to f1114d321246f9177072923e1c2a20e7aab82700: hw/watchdog/wdt_i6300esb: Use DEVICE() macro to access DeviceState.qdev (2019-06-06 11:55:04 +0200) Trivial fixes 06/06/2019 Jie Wang (1): vhost: fix incorrect print type Li Qiang (1): migration: fix a typo Markus Armbruster (1): test: Fix make target check-report.tap Philippe Mathieu-Daudé (11): docs/devel/build-system: Update an example hw/scsi/vmw_pvscsi: Use qbus_reset_all() directly hw/pci-bridge: Use the QOM BUS() macro to access BusState.qbus hw/s390x/event-facility: Use the QOM BUS() macro to access BusState.qbus hw/isa: Use the QOM DEVICE() macro to access DeviceState.qdev hw/usb-storage: Use the QOM DEVICE() macro to access DeviceState.qdev hw/vfio/pci: Use the QOM DEVICE() macro to access DeviceState.qdev hw/audio/ac97: Use the QOM DEVICE() macro to access DeviceState.qdev hw/sd: Use the QOM BUS() macro to access BusState.qbus hw/scsi: Use the QOM BUS() macro to access BusState.qbus hw/watchdog/wdt_i6300esb: Use DEVICE() macro to access DeviceState.qdev Richard Henderson (1): util: Adjust qemu_guest_getrandom_nofail for Coverity Yuval Shaia (1): hw/rdma: Delete unused headers inclusion docs/devel/build-system.txt | 19 +++ hw/audio/ac97.c | 2 +- hw/isa/lpc_ich9.c | 2 +- hw/isa/vt82c686.c | 2 +- hw/pci/pci_bridge.c | 2 +- hw/rdma/rdma_backend.c | 7 --- hw/s390x/event-facility.c | 4 ++-- hw/scsi/lsi53c895a.c| 2 +- hw/scsi/mptsas.c| 4 ++-- hw/scsi/virtio-scsi.c | 2 +- hw/scsi/vmw_pvscsi.c| 4 ++-- hw/sd/milkymist-memcard.c | 2 +- hw/sd/ssi-sd.c | 2 +- hw/usb/dev-storage.c| 2 +- hw/vfio/pci.c | 4 ++-- hw/virtio/vhost.c | 2 +- hw/watchdog/wdt_i6300esb.c | 2 +- migration/ram.c | 2 +- tests/Makefile.include | 2 +- util/guest-random.c | 2 +- 20 files changed, 29 insertions(+), 41 deletions(-) -- 2.21.0
Re: [Qemu-devel] [PATCH] qapi: InitSocketAddress: add keepalive option
On Thu, Jun 06, 2019 at 01:15:33PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
>
> Hi all!
>
> This is a continuation of "[PATCH v2 0/2] nbd: enable keepalive", but
> it's a try from another side, so almost nothing common with v2.
>
>
> qapi/sockets.json | 5 -
> util/qemu-sockets.c | 13 +
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index fc81d8d5e8..aefa024051 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -53,6 +53,8 @@
> #
> # @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
> #
> +# @keepalive: enable keepalive when connecting to this socket (Since 4.1)
> +#
> # Since: 1.3
> ##
> { 'struct': 'InetSocketAddress',
> @@ -61,7 +63,8 @@
> '*numeric': 'bool',
> '*to': 'uint16',
> '*ipv4': 'bool',
> -'*ipv6': 'bool' } }
> +'*ipv6': 'bool',
> +'*keepalive': 'bool' } }
>
> ##
> # @UnixSocketAddress:
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 8850a280a8..d2cd2a9d4f 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -457,6 +457,19 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error
> **errp)
> }
>
> freeaddrinfo(res);
> +
> +if (saddr->keepalive) {
IIUC, best practice is to use 'saddr->has_keepalive && saddr->keepalive'
> +int val = 1;
> +int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
> + &val, sizeof(val));
> +
> +if (ret < 0) {
> +error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
> +close(sock);
> +return -1;
> +}
> +}
> +
> return sock;
> }
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PULL 16/16] hw/watchdog/wdt_i6300esb: Use DEVICE() macro to access DeviceState.qdev
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a DeviceState with
"s->qdev", use the QOM prefered style: "DEVICE(s)".
This patch was generated using the following Coccinelle script:
// Use DEVICE() macros to access DeviceState.qdev
@use_device_macro_to_access_qdev@
expression obj;
identifier dev;
@@
-&obj->dev.qdev
+DEVICE(obj)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
Message-Id: <20190528164020.32250-11-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/watchdog/wdt_i6300esb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index 1c6eddf86aea..f2d1e86526f2 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -200,7 +200,7 @@ static void i6300esb_timer_expired(void *vp)
if (d->reboot_enabled) {
d->previous_reboot_flag = 1;
watchdog_perform_action(); /* This reboots, exits, etc */
-i6300esb_reset(&d->dev.qdev);
+i6300esb_reset(DEVICE(d));
}
/* In "free running mode" we start stage 1 again. */
--
2.21.0
Re: [Qemu-devel] [PATCH v9 6/6] migration: Include migration support for machine check handling
On Thursday 06 June 2019 11:36 AM, Greg Kurz wrote:
> On Thu, 6 Jun 2019 13:06:14 +1000
> David Gibson wrote:
>
>> On Wed, May 29, 2019 at 11:10:57AM +0530, Aravinda Prasad wrote:
>>> This patch includes migration support for machine check
>>> handling. Especially this patch blocks VM migration
>>> requests until the machine check error handling is
>>> complete as (i) these errors are specific to the source
>>> hardware and is irrelevant on the target hardware,
>>> (ii) these errors cause data corruption and should
>>> be handled before migration.
>>>
>>> Signed-off-by: Aravinda Prasad
>>> ---
>>> hw/ppc/spapr.c | 20
>>> hw/ppc/spapr_events.c | 17 +
>>> hw/ppc/spapr_rtas.c|4
>>> include/hw/ppc/spapr.h |2 ++
>>> 4 files changed, 43 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index e8a77636..31c4850 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -2104,6 +2104,25 @@ static const VMStateDescription vmstate_spapr_dtb = {
>>> },
>>> };
>>>
>>> +static bool spapr_fwnmi_needed(void *opaque)
>>> +{
>>> +SpaprMachineState *spapr = (SpaprMachineState *)opaque;
>>> +
>>> +return (spapr->guest_machine_check_addr == -1) ? 0 : 1;
>>
>> Since we're introducing a PAPR capability to enable this, it would
>> actually be better to check that here, rather than the runtime state.
>> That leads to less cases and easier to understand semantics for the
>> migration stream.
>>
>
> Hmmm... the purpose of needed() VMState callbacks is precisely about
> runtime state: the subsection should only be migrated if an MCE is
> pending, ie. spapr->guest_machine_check_addr != -1.
spapr->guest_machine_check_addr is set when fwnmi is registered. Hence
an MCE might not be pending if it is set.
I am fine with both the approaches (checking for
guest_machine_check_addr or for SPAPR_CAP_FWNMI_MCE).
Regards,
Aravinda
>
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_spapr_machine_check = {
>>> +.name = "spapr_machine_check",
>>> +.version_id = 1,
>>> +.minimum_version_id = 1,
>>> +.needed = spapr_fwnmi_needed,
>>> +.fields = (VMStateField[]) {
>>> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
>>> +VMSTATE_INT32(mc_status, SpaprMachineState),
>>> +VMSTATE_END_OF_LIST()
>>> +},
>>> +};
>>> +
>>> static const VMStateDescription vmstate_spapr = {
>>> .name = "spapr",
>>> .version_id = 3,
>>> @@ -2137,6 +2156,7 @@ static const VMStateDescription vmstate_spapr = {
>>> &vmstate_spapr_dtb,
>>> &vmstate_spapr_cap_large_decr,
>>> &vmstate_spapr_cap_ccf_assist,
>>> +&vmstate_spapr_machine_check,
>>> NULL
>>> }
>>> };
>>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>>> index 573c0b7..35e21e4 100644
>>> --- a/hw/ppc/spapr_events.c
>>> +++ b/hw/ppc/spapr_events.c
>>> @@ -41,6 +41,7 @@
>>> #include "qemu/bcd.h"
>>> #include "hw/ppc/spapr_ovec.h"
>>> #include
>>> +#include "migration/blocker.h"
>>>
>>> #define RTAS_LOG_VERSION_MASK 0xff00
>>> #define RTAS_LOG_VERSION_60x0600
>>> @@ -855,6 +856,22 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu,
>>> bool recovered)
>>> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>>> {
>>> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>> +int ret;
>>> +Error *local_err = NULL;
>>> +
>>> +error_setg(&spapr->fwnmi_migration_blocker,
>>> +"Live migration not supported during machine check handling");
>>> +ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err);
>>> +if (ret < 0) {
>>> +/*
>>> + * We don't want to abort and let the migration to continue. In a
>>> + * rare case, the machine check handler will run on the target
>>> + * hardware. Though this is not preferable, it is better than
>>> aborting
>>> + * the migration or killing the VM.
>>> + */
>>> +error_free(spapr->fwnmi_migration_blocker);
>>
>> You should set fwnmi_migration_blocker to NULL here as well.
>>
>> As mentioned on an earlier iteration, the migration blocker is the
>> same every time. Couldn't you just create it once and free at final
>> teardown, rather than recreating it for every NMI?
>>
>>> +warn_report_err(local_err);
>>> +}
>>>
>>> while (spapr->mc_status != -1) {
>>> /*
>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>> index 91a7ab9..c849223 100644
>>> --- a/hw/ppc/spapr_rtas.c
>>> +++ b/hw/ppc/spapr_rtas.c
>>> @@ -50,6 +50,7 @@
>>> #include "target/ppc/mmu-hash64.h"
>>> #include "target/ppc/mmu-book3s-v3.h"
>>> #include "kvm_ppc.h"
>>> +#include "migration/blocker.h"
>>>
>>> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState
>>> *spapr,
>>> uint32_t token, uint3
[Qemu-devel] [PULL 10/16] hw/isa: Use the QOM DEVICE() macro to access DeviceState.qdev
From: Philippe Mathieu-Daudé Rather than looking inside the definition of a DeviceState with "s->qdev", use the QOM prefered style: "DEVICE(s)". This patch was generated using the following Coccinelle script: // Use DEVICE() macros to access DeviceState.qdev @use_device_macro_to_access_qdev@ expression obj; identifier dev; @@ -&obj->dev.qdev +DEVICE(obj) Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Marcel Apfelbaum Message-Id: <20190528164020.32250-8-phi...@redhat.com> Signed-off-by: Laurent Vivier --- hw/isa/lpc_ich9.c | 2 +- hw/isa/vt82c686.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 031ee9cd933d..35d17246e940 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -412,7 +412,7 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled) true); } -ich9_lpc_reset(&lpc->d.qdev); +ich9_lpc_reset(DEVICE(lpc)); } /* APM */ diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 85d0532dd5b5..d46754f61cfb 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -369,7 +369,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp) pci_conf[0x90] = s->smb_io_base | 1; pci_conf[0x91] = s->smb_io_base >> 8; pci_conf[0xd2] = 0x90; -pm_smbus_init(&s->dev.qdev, &s->smb, false); +pm_smbus_init(DEVICE(s), &s->smb, false); memory_region_add_subregion(get_system_io(), s->smb_io_base, &s->smb.io); apm_init(dev, &s->apm, NULL, s); -- 2.21.0
[Qemu-devel] [PULL 15/16] hw/scsi: Use the QOM BUS() macro to access BusState.qbus
From: Philippe Mathieu-Daudé
Rather than looking inside the definition of a BusState with "s->bus.qbus",
use the QOM prefered style: "BUS(&s->bus)".
This patch was generated using the following Coccinelle script:
// Use BUS() macros to access BusState.qbus
@use_bus_macro_to_access_qbus@
expression obj;
identifier bus;
@@
-&obj->bus.qbus
+BUS(&obj->bus)
Suggested-by: Peter Maydell
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
Message-Id: <20190528164020.32250-3-phi...@redhat.com>
Signed-off-by: Laurent Vivier
---
hw/scsi/lsi53c895a.c | 2 +-
hw/scsi/mptsas.c | 4 ++--
hw/scsi/virtio-scsi.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index da7239d94fcd..a8b7a199f960 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1860,7 +1860,7 @@ static void lsi_reg_writeb(LSIState *s, int offset,
uint8_t val)
}
if (val & LSI_SCNTL1_RST) {
if (!(s->sstat0 & LSI_SSTAT0_RST)) {
-qbus_reset_all(&s->bus.qbus);
+qbus_reset_all(BUS(&s->bus));
s->sstat0 |= LSI_SSTAT0_RST;
lsi_script_scsi_interrupt(s, LSI_SIST0_RST, 0);
}
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 929404fb48c8..e800683e91e9 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -540,7 +540,7 @@ reply_maybe_async:
break;
case MPI_SCSITASKMGMT_TASKTYPE_RESET_BUS:
-qbus_reset_all(&s->bus.qbus);
+qbus_reset_all(BUS(&s->bus));
break;
default:
@@ -803,7 +803,7 @@ static void mptsas_soft_reset(MPTSASState *s)
s->intr_mask = MPI_HIM_DIM | MPI_HIM_RIM;
mptsas_update_interrupt(s);
-qbus_reset_all(&s->bus.qbus);
+qbus_reset_all(BUS(&s->bus));
s->intr_status = 0;
s->intr_mask = save_mask;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 2994f0738fd8..12d21bbc613c 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -696,7 +696,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
assert(!s->dataplane_started);
s->resetting++;
-qbus_reset_all(&s->bus.qbus);
+qbus_reset_all(BUS(&s->bus));
s->resetting--;
vs->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
--
2.21.0
[Qemu-devel] [PULL 02/16] migration: fix a typo
From: Li Qiang 'postocpy' should be 'postcopy'. CC: qemu-triv...@nongnu.org Signed-off-by: Li Qiang Reviewed-by: Alex Bennée Reviewed-by: Juan Quintela Message-Id: <20190525062832.18009-1-liq...@163.com> Signed-off-by: Laurent Vivier --- migration/ram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/ram.c b/migration/ram.c index 4c608692262e..359dc8f1bc44 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2236,7 +2236,7 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset) } /** - * get_queued_page: unqueue a page from the postocpy requests + * get_queued_page: unqueue a page from the postcopy requests * * Skips pages that are already sent (!dirty) * -- 2.21.0
Re: [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value
On Jun 5, 2019 11:13 PM, "Richard Henderson"
wrote:
>
> The kernel will return -EINVAL for bits set in the prot argument
> that are unknown or invalid. Previously we were simply cropping
> out the bits that we care about.
>
> Introduce validate_prot_to_pageflags to perform this check in a
> single place between the two syscalls. Differentiate between
> the target and host versions of prot. Compute the qemu internal
> page_flags value at the same time.
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/mmap.c | 106 --
> 1 file changed, 75 insertions(+), 31 deletions(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index af41339d57..3117f57fd8 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -61,11 +61,38 @@ void mmap_fork_end(int child)
> pthread_mutex_unlock(&mmap_mutex);
> }
>
> +/*
> + * Validate target prot bitmask.
> + * Return the prot bitmask for the host in *HOST_PROT.
> + * Return 0 if the target prot bitmask is invalid, otherwise
> + * the internal qemu page_flags (which will include PAGE_VALID).
> + */
> +static int validate_prot_to_pageflags(int *host_prot, int prot)
> +{
> +int valid = PROT_READ | PROT_WRITE | PROT_EXEC | TARGET_PROT_SEM;
> +int page_flags = (prot & PAGE_BITS) | PAGE_VALID;
> +
> +/*
> + * For the host, we need not pass anything except read/write/exec.
> + * While PROT_SEM is allowed by all hosts, it is also ignored, so
> + * don't bother transforming guest bit to host bit.
I don't think that making assumptions based on an undocumented behavior is
the best practice.
Your “Let's don't bother” about such easy to implement things may create a
lot of bothering in future.
Regards,
Aleksandar
> Any other
> + * target-specific prot bits will not be understood by the host
> + * and will need to be encoded into page_flags for qemu emulation.
> + *
> + * TODO: We do not actually have to map guest pages as executable,
> + * since they will not be directly executed by the host. We only
> + * need to remember exec within page_flags.
> + */
> +*host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
> +
> +return prot & ~valid ? 0 : page_flags;
> +}
> +
> /* NOTE: all the constants are the HOST ones, but addresses are target.
*/
> -int target_mprotect(abi_ulong start, abi_ulong len, int prot)
> +int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
> {
> abi_ulong end, host_start, host_end, addr;
> -int prot1, ret;
> +int prot1, ret, page_flags, host_prot;
>
> #ifdef DEBUG_MMAP
> printf("mprotect: start=0x" TARGET_ABI_FMT_lx
> @@ -75,56 +102,65 @@ int target_mprotect(abi_ulong start, abi_ulong len,
int prot)
> prot & PROT_EXEC ? 'x' : '-');
> #endif
>
> -if ((start & ~TARGET_PAGE_MASK) != 0)
> +if ((start & ~TARGET_PAGE_MASK) != 0) {
> return -TARGET_EINVAL;
> +}
> +page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
> +if (!page_flags) {
> +return -TARGET_EINVAL;
> +}
> len = TARGET_PAGE_ALIGN(len);
> end = start + len;
> if (!guest_range_valid(start, len)) {
> return -TARGET_ENOMEM;
> }
> -prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
> -if (len == 0)
> +if (len == 0) {
> return 0;
> +}
>
> mmap_lock();
> host_start = start & qemu_host_page_mask;
> host_end = HOST_PAGE_ALIGN(end);
> if (start > host_start) {
> /* handle host page containing start */
> -prot1 = prot;
> -for(addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
> +prot1 = host_prot;
> +for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
> prot1 |= page_get_flags(addr);
> }
> if (host_end == host_start + qemu_host_page_size) {
> -for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> +for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> prot1 |= page_get_flags(addr);
> }
> end = host_end;
> }
> -ret = mprotect(g2h(host_start), qemu_host_page_size, prot1 &
PAGE_BITS);
> -if (ret != 0)
> +ret = mprotect(g2h(host_start), qemu_host_page_size,
> + prot1 & PAGE_BITS);
> +if (ret != 0) {
> goto error;
> +}
> host_start += qemu_host_page_size;
> }
> if (end < host_end) {
> -prot1 = prot;
> -for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> +prot1 = host_prot;
> +for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> prot1 |= page_get_flags(addr);
> }
> -ret = mprotect(g2h(host_end - qemu_host_page_size),
qemu_host_page_size,
> - prot1 & PAGE_BITS);
> -if (ret != 0)
> +ret = mprotect(g2h(host_end - qemu_host_p
Re: [Qemu-devel] [PATCH v9 6/6] migration: Include migration support for machine check handling
On Thursday 06 June 2019 08:36 AM, David Gibson wrote:
> On Wed, May 29, 2019 at 11:10:57AM +0530, Aravinda Prasad wrote:
>> This patch includes migration support for machine check
>> handling. Especially this patch blocks VM migration
>> requests until the machine check error handling is
>> complete as (i) these errors are specific to the source
>> hardware and is irrelevant on the target hardware,
>> (ii) these errors cause data corruption and should
>> be handled before migration.
>>
>> Signed-off-by: Aravinda Prasad
>> ---
>> hw/ppc/spapr.c | 20
>> hw/ppc/spapr_events.c | 17 +
>> hw/ppc/spapr_rtas.c|4
>> include/hw/ppc/spapr.h |2 ++
>> 4 files changed, 43 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index e8a77636..31c4850 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2104,6 +2104,25 @@ static const VMStateDescription vmstate_spapr_dtb = {
>> },
>> };
>>
>> +static bool spapr_fwnmi_needed(void *opaque)
>> +{
>> +SpaprMachineState *spapr = (SpaprMachineState *)opaque;
>> +
>> +return (spapr->guest_machine_check_addr == -1) ? 0 : 1;
>
> Since we're introducing a PAPR capability to enable this, it would
> actually be better to check that here, rather than the runtime state.
> That leads to less cases and easier to understand semantics for the
> migration stream.
I am fine with this approach as well.
>
>> +}
>> +
>> +static const VMStateDescription vmstate_spapr_machine_check = {
>> +.name = "spapr_machine_check",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.needed = spapr_fwnmi_needed,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
>> +VMSTATE_INT32(mc_status, SpaprMachineState),
>> +VMSTATE_END_OF_LIST()
>> +},
>> +};
>> +
>> static const VMStateDescription vmstate_spapr = {
>> .name = "spapr",
>> .version_id = 3,
>> @@ -2137,6 +2156,7 @@ static const VMStateDescription vmstate_spapr = {
>> &vmstate_spapr_dtb,
>> &vmstate_spapr_cap_large_decr,
>> &vmstate_spapr_cap_ccf_assist,
>> +&vmstate_spapr_machine_check,
>> NULL
>> }
>> };
>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>> index 573c0b7..35e21e4 100644
>> --- a/hw/ppc/spapr_events.c
>> +++ b/hw/ppc/spapr_events.c
>> @@ -41,6 +41,7 @@
>> #include "qemu/bcd.h"
>> #include "hw/ppc/spapr_ovec.h"
>> #include
>> +#include "migration/blocker.h"
>>
>> #define RTAS_LOG_VERSION_MASK 0xff00
>> #define RTAS_LOG_VERSION_60x0600
>> @@ -855,6 +856,22 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu,
>> bool recovered)
>> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>> {
>> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> +int ret;
>> +Error *local_err = NULL;
>> +
>> +error_setg(&spapr->fwnmi_migration_blocker,
>> +"Live migration not supported during machine check handling");
>> +ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err);
>> +if (ret < 0) {
>> +/*
>> + * We don't want to abort and let the migration to continue. In a
>> + * rare case, the machine check handler will run on the target
>> + * hardware. Though this is not preferable, it is better than
>> aborting
>> + * the migration or killing the VM.
>> + */
>> +error_free(spapr->fwnmi_migration_blocker);
>
> You should set fwnmi_migration_blocker to NULL here as well.
ok.
>
> As mentioned on an earlier iteration, the migration blocker is the
> same every time. Couldn't you just create it once and free at final
> teardown, rather than recreating it for every NMI?
That means, we create the error string at the time when ibm,nmi-register
is invoked and tear it down during machine reset?
Regards,
Aravinda
>
>> +warn_report_err(local_err);
>> +}
>>
>> while (spapr->mc_status != -1) {
>> /*
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 91a7ab9..c849223 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -50,6 +50,7 @@
>> #include "target/ppc/mmu-hash64.h"
>> #include "target/ppc/mmu-book3s-v3.h"
>> #include "kvm_ppc.h"
>> +#include "migration/blocker.h"
>>
>> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState
>> *spapr,
>> uint32_t token, uint32_t nargs,
>> @@ -404,6 +405,9 @@ static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> spapr->mc_status = -1;
>> qemu_cond_signal(&spapr->mc_delivery_cond);
>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +migrate_del_blocker(spapr->fwnmi_migration_blocker);
>> +error_free(spapr->fwnmi_migration_blocker);
>> +spapr->fwnmi_migration_blocker = NULL;
>> }
>> }
>>
>> dif
Re: [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code
Kevin Wolf writes: > Documentation comments follow a certain structure: First, we have a text > with a general description (called QAPIDoc.body). After this, > descriptions of the arguments follow. Finally, we have a part that > contains various named sections. > > The code doesn't show this structure, but just checks various attributes > that indicate indirectly which part is being processed, so it happens to > do the right set of things in the right phase. This is hard to follow, > and adding support for documentation of features would be even harder. > > This patch restructures the code so that the three parts are clearly > separated. The code becomes a bit longer, but easier to follow. The > resulting output remains unchanged. > > Signed-off-by: Kevin Wolf > --- > scripts/qapi/common.py | 122 - > 1 file changed, 97 insertions(+), 25 deletions(-) > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 9e4b6c00b5..55ccd216cf 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -120,6 +120,22 @@ class QAPIDoc(object): > def connect(self, member): > self.member = member > > +class DocPart: > +""" Did you drop the "A documentation part" headline intentionally? > +Expression documentation blocks consist of > +* a BODY part: first line naming the expression, plus an > + optional overview > +* an ARGS part: description of each argument (for commands and > + events) or member (for structs, unions and alternates), > +* a VARIOUS part: optional tagged sections. > + > +Free-form documentation blocks consist only of a BODY part. > +""" > +# TODO Make it a subclass of Enum when Python 2 support is removed > +BODY = 1 > +ARGS = 2 > +VARIOUS = 3 > + > def __init__(self, parser, info): > # self._parser is used to report errors with QAPIParseError. The > # resulting error position depends on the state of the parser. > @@ -135,6 +151,7 @@ class QAPIDoc(object): > self.sections = [] > # the current section > self._section = self.body > +self._part = QAPIDoc.DocPart.BODY > > def has_section(self, name): > """Return True if we have a section with this name.""" > @@ -144,7 +161,24 @@ class QAPIDoc(object): > return False > > def append(self, line): > -"""Parse a comment line and add it to the documentation.""" > +""" > +Parse a comment line and add it to the documentation. > + > +The way that the line is dealt with depends on which part of the > +documentation we're parsing right now: > + > +BODY means that we're ready to process free-form text into > self.body. A > +symbol name is only allowed if no other text was parsed yet. It is > +interpreted as the symbol name that describes the currently > documented > +object. On getting the second symbol name, we proceed to ARGS. > + > +ARGS means that we're parsing the arguments section. Any symbol name > is > +interpreted as an argument and an ArgSection is created for it. > + > +VARIOUS is the final part where free-form sections may appear. This > +includes named sections such as "Return:" as well as unnamed > +paragraphs. Symbols are not allowed any more in this part. > +""" A few little things: * The reader has to make the connection from BODY, ARGS, VARIOUS to self._part. To help him a bit, I'd say something like "depends on which part of the documentation we're parsing right now, according to self._part:" * In the paragraph on BODY: on first reading, "A symbol name is only allowed if no other text was parsed yet" appears to contradict "On getting the second symbol name". It doesn't: the second symbol name doesn't belong to this part. Perhaps we could phrase this more clearly. Not sure it's worth the trouble. * PEP 8: "For flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters." * PEP 8: "You should use two spaces after a sentence-ending period in multi-sentence comments, except after the final sentence." > line = line[1:] > if not line: > self._append_freeform(line) > @@ -154,37 +188,85 @@ class QAPIDoc(object): > raise QAPIParseError(self._parser, "Missing space after #") > line = line[1:] > > +if self._part == QAPIDoc.DocPart.BODY: > +self._append_body_line(line) > +elif self._part == QAPIDoc.DocPart.ARGS: > +self._append_args_line(line) > +elif self._part == QAPIDoc.DocPart.VARIOUS: > +self._append_various_line(line) > +else: > +assert False In my review of v2, I suggested to use a function-valued vari
Re: [Qemu-devel] QEMU/KVM migration backwards compatibility broken?
> On 6 Jun 2019, at 14:07, Dr. David Alan Gilbert wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> >> >>> On 6 Jun 2019, at 13:39, Dr. David Alan Gilbert wrote: >>> >>> * Liran Alon (liran.a...@oracle.com) wrote: > On 6 Jun 2019, at 12:23, Dr. David Alan Gilbert > wrote: > > * Liran Alon (liran.a...@oracle.com) wrote: >> >> >>> On 6 Jun 2019, at 11:42, Dr. David Alan Gilbert >>> wrote: >>> >>> * Liran Alon (liran.a...@oracle.com) wrote: Hi, Looking at QEMU source code, I am puzzled regarding how migration backwards compatibility is preserved regarding X86CPU. As I understand it, fields that are based on KVM capabilities and guest runtime usage are defined in VMState subsections in order to not send them if not necessary. This is done such that in case they are not needed and we migrate to an old QEMU which don’t support loading this state, migration will still succeed (As .needed() method will return false and therefore this state won’t be sent as part of migration stream). Furthermore, in case .needed() returns true and old QEMU don’t support loading this state, migration fails. As it should because we are aware that guest state is not going to be restored properly on destination. I’m puzzled about what will happen in the following scenario: 1) Source is running new QEMU with new KVM that supports save of some VMState subsection. 2) Destination is running new QEMU that supports load this state but with old kernel that doesn’t know how to load this state. I would have expected in this case that if source .needed() returns true, then migration will fail because of lack of support in destination kernel. However, it seems from current QEMU code that this will actually succeed in many cases. For example, if msr_smi_count is sent as part of migration stream (See vmstate_msr_smi_count) and destination have has_msr_smi_count==false, then destination will succeed loading migration stream but kvm_put_msrs() will actually ignore env->msr_smi_count and will successfully load guest state. Therefore, migration will succeed even though it should have failed… It seems to me that QEMU should have for every such VMState subsection, a .post_load() method that verifies that relevant capability is supported by kernel and otherwise fail migration. What do you think? Should I really create a patch to modify all these CPUX86 VMState subsections to behave like this? >>> >>> I don't know the x86 specific side that much; but from my migration side >>> the answer should mostly be through machine types - indeed for smi-count >>> there's a property 'x-migrate-smi-count' which is off for machine types >>> pre 2.11 (see hw/i386/pc.c pc_compat_2_11) - so if you've got an old >>> kernel you should stick to the old machine types. >>> >>> There's nothing guarding running the new machine type on old-kernels; >>> and arguably we should have a check at startup that complains if >>> your kernel is missing something the machine type uses. >>> However, that would mean that people running with -M pc would fail >>> on old kernels. >>> >>> A post-load is also a valid check; but one question is whether, >>> for a particular register, the pain is worth it - it depends on the >>> symptom that the missing state causes. If it's minor then you might >>> conclude it's not worth a failed migration; if it's a hung or >>> corrupt guest then yes it is. Certainly a warning printed is worth >>> it. >>> >>> Dave >> >> I think we should have flags that allow user to specify which VMState >> subsections user explicitly allow to avoid restore even though they are >> required to fully restore guest state. >> But it seems to me that the behaviour should be to always fail migration >> in case we load a VMState subsections that we are unable to restore >> unless user explicitly specified this is ok >> for this specific subsection. >> Therefore, it seems that for every VMState subsection that it’s restore >> is based on kernel capability we should: >> 1) Have a user-controllable flag (which is also tied to machine-type?) >> to explicitly allow avoid restoring this state if cannot. Default should >> be “false”. >> 2) Have a .post_load() method that verifies we have required kernel >> capability to restore this state, unless flag (1) was specified as >> “true”. > > This seems a lot of flags; users aren't goin
Re: [Qemu-devel] [PATCH] qapi: InitSocketAddress: add keepalive option
06.06.2019 14:17, Daniel P. Berrangé wrote:
> On Thu, Jun 06, 2019 at 01:15:33PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy
>> ---
>>
>> Hi all!
>>
>> This is a continuation of "[PATCH v2 0/2] nbd: enable keepalive", but
>> it's a try from another side, so almost nothing common with v2.
>>
>>
>> qapi/sockets.json | 5 -
>> util/qemu-sockets.c | 13 +
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/sockets.json b/qapi/sockets.json
>> index fc81d8d5e8..aefa024051 100644
>> --- a/qapi/sockets.json
>> +++ b/qapi/sockets.json
>> @@ -53,6 +53,8 @@
>> #
>> # @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
>> #
>> +# @keepalive: enable keepalive when connecting to this socket (Since 4.1)
>> +#
>> # Since: 1.3
>> ##
>> { 'struct': 'InetSocketAddress',
>> @@ -61,7 +63,8 @@
>> '*numeric': 'bool',
>> '*to': 'uint16',
>> '*ipv4': 'bool',
>> -'*ipv6': 'bool' } }
>> +'*ipv6': 'bool',
>> +'*keepalive': 'bool' } }
>>
>> ##
>> # @UnixSocketAddress:
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 8850a280a8..d2cd2a9d4f 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -457,6 +457,19 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error
>> **errp)
>> }
>>
>> freeaddrinfo(res);
>> +
>> +if (saddr->keepalive) {
>
> IIUC, best practice is to use 'saddr->has_keepalive && saddr->keepalive'
As I remember, now all optional fields are zeroed. But I'm not against stricter
condition.
>
>> +int val = 1;
>> +int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
>> + &val, sizeof(val));
>> +
>> +if (ret < 0) {
>> +error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
>> +close(sock);
>> +return -1;
>> +}
>> +}
>> +
>> return sock;
>> }
>
> Regards,
> Daniel
>
--
Best regards,
Vladimir
Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/9] configure: permit use of io_uring
On Mon, 2019-06-03 at 18:08 +0530, Aarushi Mehta wrote: > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Aarushi Mehta > --- > configure | 27 +++ > 1 file changed, 27 insertions(+) > > diff --git a/configure b/configure > index 6cdcfb2dc3..cb49ef0fcc 100755 > --- a/configure > +++ b/configure > @@ -365,6 +365,7 @@ xen="" > xen_ctrl_version="" > xen_pci_passthrough="" > linux_aio="" > +linux_io_uring="" > cap_ng="" > attr="" > libattr="" > @@ -1266,6 +1267,10 @@ for opt do >;; >--enable-linux-aio) linux_aio="yes" >;; > + --disable-linux-io-uring) linux_io_uring="no" > + ;; > + --enable-linux-io-uring) linux_io_uring="yes" > + ;; >--disable-attr) attr="no" >;; >--enable-attr) attr="yes" > @@ -1784,6 +1789,7 @@ disabled with --disable-FEATURE, default is enabled if > available: >vde support for vde network >netmap support for netmap network >linux-aio Linux AIO support > + linux-io-uring Linux io_uring support >cap-ng libcap-ng support >attrattr and xattr support >vhost-net vhost-net kernel acceleration support > @@ -3973,6 +3979,21 @@ EOF > linux_aio=no >fi > fi > +## > +# linux-io-uring probe > + > +if test "$linux_io_uring" != "no" ; then > + if $pkg_config liburing; then > +linux_io_uring_cflags=$($pkg_config --cflags liburing) > +linux_io_uring_libs=$($pkg_config --libs liburing) > +linux_io_uring=yes > + else > +if test "$linux_io_uring" = "yes" ; then > + feature_not_found "linux io_uring" "Install liburing devel" > +fi > +linux_io_uring=no > + fi > +fi > > ## > # TPM emulation is only on POSIX > @@ -6396,6 +6417,7 @@ echo "PIE $pie" > echo "vde support $vde" > echo "netmap support$netmap" > echo "Linux AIO support $linux_aio" > +echo "Linux io_uring support $linux_io_uring" > echo "ATTR/XATTR support $attr" > echo "Install blobs $blobs" > echo "KVM support $kvm" > @@ -6876,6 +6898,11 @@ fi > if test "$linux_aio" = "yes" ; then >echo "CONFIG_LINUX_AIO=y" >> $config_host_mak > fi > +if test "$linux_io_uring" = "yes" ; then > + echo "CONFIG_LINUX_IO_URING=y" >> $config_host_mak > + echo "LINUX_IO_URING_CFLAGS=$linux_io_uring_cflags" >> $config_host_mak > + echo "LINUX_IO_URING_LIBS=$linux_io_uring_libs" >> $config_host_mak > +fi > if test "$attr" = "yes" ; then >echo "CONFIG_ATTR=y" >> $config_host_mak > fi Reviewed-by: Maxim Levitsky Best regards, Maxim Levitsky
Re: [Qemu-devel] [Qemu-block] [PATCH v4 2/9] qapi/block-core: add option for io_uring
On Wed, 2019-06-05 at 07:58 +0200, Markus Armbruster wrote:
> Aarushi Mehta writes:
>
> > Option only enumerates for hosts that support it.
>
> Blank line here, please. Same in other patches.
>
> > Signed-off-by: Aarushi Mehta
> > ---
> > qapi/block-core.json | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 1defcde048..db7eedd058 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2792,11 +2792,13 @@
> > #
> > # @threads: Use qemu's thread pool
> > # @native: Use native AIO backend (only Linux and Windows)
> > +# @io_uring:Use linux io_uring (since 4.1)
> > #
> > # Since: 2.9
> > ##
> > { 'enum': 'BlockdevAioOptions',
> > - 'data': [ 'threads', 'native' ] }
> > + 'data': [ 'threads', 'native',
> > +{ 'name': 'io_uring', 'if': 'defined(CONFIG_LINUX_IO_URING)' }
> > ] }
>
> We prefer '-' over '_' in the QAPI schema: 'io-uring' instead of
> 'io_uring'. Exceptions can be made when existing siblings use '_' (not
> the case here), or to match how the thing is commonly spelled outside
> QEMU. Up to the subject matter experts; I just want to make sure it's
> not accidental
I agree with that.
Other than that,
Reviewed-by: Maxim Levitsky
Best regards,
Maxim Levitsky
Re: [Qemu-devel] [PULL 0/2] SCSI/vl.c patches for 2019-06-05
On Wed, 5 Jun 2019 at 15:45, Paolo Bonzini wrote: > > The following changes since commit c87759ce876a7a0b17c2bf4f0b964bd51f0ee871: > > q35: Revert to kernel irqchip (2019-06-03 14:03:03 +0200) > > are available in the git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to edfb4389c26cbfd873707306024130bda6049780: > > vl: Document why objects are delayed (2019-06-05 16:15:32 +0200) > > > * Fix pr-manager-helper (Markus) > > > Markus Armbruster (2): > vl: Fix -drive / -blockdev persistent reservation management > vl: Document why objects are delayed > > vl.c | 13 + Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1 for any user-visible changes. -- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH v4 3/9] block/block: add BDRV flag for io_uring
On Mon, 2019-06-03 at 18:08 +0530, Aarushi Mehta wrote:
> Signed-off-by: Aarushi Mehta
> Reviewed-by: Stefan Hajnoczi
> ---
> include/block/block.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 9b083e2bca..60f7c6c01c 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -121,6 +121,7 @@ typedef struct HDGeometry {
>ignoring the format layer */
> #define BDRV_O_NO_IO 0x1 /* don't initialize for I/O */
> #define BDRV_O_AUTO_RDONLY 0x2 /* degrade to read-only if opening
> read-write fails */
> +#define BDRV_O_IO_URING0x4 /* use io_uring instead of the thread
> pool */
>
> #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
>
I had some fun learning now why do we need that flag.
Lot of code could be removed when someday we will remove the -drive interface.
Reviewed-by: Maxim Levitsky
Best regards,
Maxim Levitsky
[Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments
Signed-off-by: Markus Armbruster --- This is on top of the fixup I appended to my review of v4. I'd squash all three patches together. The next patch needs to be updated for this. Unsquashed branch at git://repo.or.cz/qemu/armbru.git branch qapi-features. Let me know what you think. scripts/qapi/common.py | 109 - 1 file changed, 63 insertions(+), 46 deletions(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index b42dad9b36..004b68df5c 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -102,6 +102,22 @@ class QAPISemError(QAPIError): class QAPIDoc(object): +""" +A documentation comment block, either expression or free-form + +Expression documentation blocks consist of + +* a body section: one line naming the expression, followed by an + overview (any number of lines) + +* argument sections: a description of each argument (for commands + and events) or member (for structs, unions and alternates) + +* additional (non-argument) sections, possibly tagged + +Free-form documentation blocks consist only of a body section. +""" + class Section(object): def __init__(self, name=None): # optional section name (argument/member or section name) @@ -120,22 +136,6 @@ class QAPIDoc(object): def connect(self, member): self.member = member -class DocPart: -""" -Expression documentation blocks consist of -* a BODY part: first line naming the expression, plus an - optional overview -* an ARGS part: description of each argument (for commands and - events) or member (for structs, unions and alternates), -* a VARIOUS part: optional tagged sections. - -Free-form documentation blocks consist only of a BODY part. -""" -# TODO Make it a subclass of Enum when Python 2 support is removed -BODY = 1 -ARGS = 2 -VARIOUS = 3 - def __init__(self, parser, info): # self._parser is used to report errors with QAPIParseError. The # resulting error position depends on the state of the parser. @@ -151,7 +151,7 @@ class QAPIDoc(object): self.sections = [] # the current section self._section = self.body -self._part = QAPIDoc.DocPart.BODY +self._append_line = self._append_body_line def has_section(self, name): """Return True if we have a section with this name.""" @@ -164,20 +164,11 @@ class QAPIDoc(object): """ Parse a comment line and add it to the documentation. -The way that the line is dealt with depends on which part of the -documentation we're parsing right now: - -BODY means that we're ready to process free-form text into self.body. A -symbol name is only allowed if no other text was parsed yet. It is -interpreted as the symbol name that describes the currently documented -object. On getting the second symbol name, we proceed to ARGS. - -ARGS means that we're parsing the arguments section. Any symbol name is -interpreted as an argument and an ArgSection is created for it. - -VARIOUS is the final part where free-form sections may appear. This -includes named sections such as "Return:" as well as unnamed -paragraphs. Symbols are not allowed any more in this part. +The way that the line is dealt with depends on which part of +the documentation we're parsing right now: +* The body section: ._append_line is ._append_body_line +* An argument section: ._append_line is ._append_args_line +* An additional section: ._append_line is ._append_various_line """ line = line[1:] if not line: @@ -187,15 +178,7 @@ class QAPIDoc(object): if line[0] != ' ': raise QAPIParseError(self._parser, "Missing space after #") line = line[1:] - -if self._part == QAPIDoc.DocPart.BODY: -self._append_body_line(line) -elif self._part == QAPIDoc.DocPart.ARGS: -self._append_args_line(line) -elif self._part == QAPIDoc.DocPart.VARIOUS: -self._append_various_line(line) -else: -assert False +self._append_line(line) def end_comment(self): self._end_section() @@ -209,6 +192,19 @@ class QAPIDoc(object): 'TODO:') def _append_body_line(self, line): +""" +Process a line of documentation text in the body section. + +If this a symbol line and it is the section's first line, this +is an expression documentation block for that symbol. + +If it's an expression documentation block, another symbol line +begins the argument section for the argument named by it, and +a section tag begins an additional section. Start that +section and
Re: [Qemu-devel] [PATCH v9 6/6] migration: Include migration support for machine check handling
On Thu, 6 Jun 2019 16:45:30 +0530
Aravinda Prasad wrote:
> On Thursday 06 June 2019 11:36 AM, Greg Kurz wrote:
> > On Thu, 6 Jun 2019 13:06:14 +1000
> > David Gibson wrote:
> >
> >> On Wed, May 29, 2019 at 11:10:57AM +0530, Aravinda Prasad wrote:
> >>> This patch includes migration support for machine check
> >>> handling. Especially this patch blocks VM migration
> >>> requests until the machine check error handling is
> >>> complete as (i) these errors are specific to the source
> >>> hardware and is irrelevant on the target hardware,
> >>> (ii) these errors cause data corruption and should
> >>> be handled before migration.
> >>>
> >>> Signed-off-by: Aravinda Prasad
> >>> ---
> >>> hw/ppc/spapr.c | 20
> >>> hw/ppc/spapr_events.c | 17 +
> >>> hw/ppc/spapr_rtas.c|4
> >>> include/hw/ppc/spapr.h |2 ++
> >>> 4 files changed, 43 insertions(+)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index e8a77636..31c4850 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -2104,6 +2104,25 @@ static const VMStateDescription vmstate_spapr_dtb
> >>> = {
> >>> },
> >>> };
> >>>
> >>> +static bool spapr_fwnmi_needed(void *opaque)
> >>> +{
> >>> +SpaprMachineState *spapr = (SpaprMachineState *)opaque;
> >>> +
> >>> +return (spapr->guest_machine_check_addr == -1) ? 0 : 1;
> >>
> >> Since we're introducing a PAPR capability to enable this, it would
> >> actually be better to check that here, rather than the runtime state.
> >> That leads to less cases and easier to understand semantics for the
> >> migration stream.
> >>
> >
> > Hmmm... the purpose of needed() VMState callbacks is precisely about
> > runtime state: the subsection should only be migrated if an MCE is
> > pending, ie. spapr->guest_machine_check_addr != -1.
>
> spapr->guest_machine_check_addr is set when fwnmi is registered. Hence
> an MCE might not be pending if it is set.
>
Oops sorry, got confused... should have written "if the guest has
registered FWNMI", but the idea is the same. We only need to migrate
the subsection if the state is different from reset. This is the way
needed() callbacks are generally implemented.
> I am fine with both the approaches (checking for
> guest_machine_check_addr or for SPAPR_CAP_FWNMI_MCE).
>
I would find ackward to migrate this always for new machine types,
even if the guest doesn't register FWNMI...
> Regards,
> Aravinda
>
> >
> >>> +}
> >>> +
> >>> +static const VMStateDescription vmstate_spapr_machine_check = {
> >>> +.name = "spapr_machine_check",
> >>> +.version_id = 1,
> >>> +.minimum_version_id = 1,
> >>> +.needed = spapr_fwnmi_needed,
> >>> +.fields = (VMStateField[]) {
> >>> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
> >>> +VMSTATE_INT32(mc_status, SpaprMachineState),
> >>> +VMSTATE_END_OF_LIST()
> >>> +},
> >>> +};
> >>> +
> >>> static const VMStateDescription vmstate_spapr = {
> >>> .name = "spapr",
> >>> .version_id = 3,
> >>> @@ -2137,6 +2156,7 @@ static const VMStateDescription vmstate_spapr = {
> >>> &vmstate_spapr_dtb,
> >>> &vmstate_spapr_cap_large_decr,
> >>> &vmstate_spapr_cap_ccf_assist,
> >>> +&vmstate_spapr_machine_check,
> >>> NULL
> >>> }
> >>> };
> >>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> >>> index 573c0b7..35e21e4 100644
> >>> --- a/hw/ppc/spapr_events.c
> >>> +++ b/hw/ppc/spapr_events.c
> >>> @@ -41,6 +41,7 @@
> >>> #include "qemu/bcd.h"
> >>> #include "hw/ppc/spapr_ovec.h"
> >>> #include
> >>> +#include "migration/blocker.h"
> >>>
> >>> #define RTAS_LOG_VERSION_MASK 0xff00
> >>> #define RTAS_LOG_VERSION_60x0600
> >>> @@ -855,6 +856,22 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu,
> >>> bool recovered)
> >>> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
> >>> {
> >>> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >>> +int ret;
> >>> +Error *local_err = NULL;
> >>> +
> >>> +error_setg(&spapr->fwnmi_migration_blocker,
> >>> +"Live migration not supported during machine check
> >>> handling");
> >>> +ret = migrate_add_blocker(spapr->fwnmi_migration_blocker,
> >>> &local_err);
> >>> +if (ret < 0) {
> >>> +/*
> >>> + * We don't want to abort and let the migration to continue. In a
> >>> + * rare case, the machine check handler will run on the target
> >>> + * hardware. Though this is not preferable, it is better than
> >>> aborting
> >>> + * the migration or killing the VM.
> >>> + */
> >>> +error_free(spapr->fwnmi_migration_blocker);
> >>
> >> You should set fwnmi_migration_blocker to NULL here as well.
> >>
> >> As mentioned on an earlier iteration, the migration blocker is the
> >> same every time. Couldn't
Re: [Qemu-devel] [PULL 09/16] hw/s390x/event-facility: Use the QOM BUS() macro to access BusState.qbus
On 06.06.19 13:06, Laurent Vivier wrote: > From: Philippe Mathieu-Daudé > > Rather than looking inside the definition of a BusState with "s->bus.qbus", > use the QOM prefered style: "BUS(&s->bus)". > > This patch was generated using the following Coccinelle script: > > // Use BUS() macros to access BusState.qbus > @use_bus_macro_to_access_qbus@ > expression obj; > identifier bus; > @@ > -&obj->bus.qbus > +BUS(&obj->bus) > > Suggested-by: Peter Maydell > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Cornelia Huck > Message-Id: <20190528164020.32250-5-phi...@redhat.com> > Signed-off-by: Laurent Vivier > --- > hw/s390x/event-facility.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index ee5b83448b11..e574223a226b 100644 > --- a/hw/s390x/event-facility.c > +++ b/hw/s390x/event-facility.c > @@ -466,12 +466,12 @@ static void init_event_facility(Object *obj) > new = object_new(TYPE_SCLP_QUIESCE); > object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL); > object_unref(new); > -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); > > new = object_new(TYPE_SCLP_CPU_HOTPLUG); > object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL); > object_unref(new); > -qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > +qdev_set_parent_bus(DEVICE(new), BUS(&event_facility->sbus)); > /* the facility will automatically realize the devices via the bus */ > } > > Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v9 6/6] migration: Include migration support for machine check handling
On Thu, 6 Jun 2019 16:55:18 +0530
Aravinda Prasad wrote:
> On Thursday 06 June 2019 08:36 AM, David Gibson wrote:
> > On Wed, May 29, 2019 at 11:10:57AM +0530, Aravinda Prasad wrote:
> >> This patch includes migration support for machine check
> >> handling. Especially this patch blocks VM migration
> >> requests until the machine check error handling is
> >> complete as (i) these errors are specific to the source
> >> hardware and is irrelevant on the target hardware,
> >> (ii) these errors cause data corruption and should
> >> be handled before migration.
> >>
> >> Signed-off-by: Aravinda Prasad
> >> ---
> >> hw/ppc/spapr.c | 20
> >> hw/ppc/spapr_events.c | 17 +
> >> hw/ppc/spapr_rtas.c|4
> >> include/hw/ppc/spapr.h |2 ++
> >> 4 files changed, 43 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index e8a77636..31c4850 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -2104,6 +2104,25 @@ static const VMStateDescription vmstate_spapr_dtb =
> >> {
> >> },
> >> };
> >>
> >> +static bool spapr_fwnmi_needed(void *opaque)
> >> +{
> >> +SpaprMachineState *spapr = (SpaprMachineState *)opaque;
> >> +
> >> +return (spapr->guest_machine_check_addr == -1) ? 0 : 1;
> >
> > Since we're introducing a PAPR capability to enable this, it would
> > actually be better to check that here, rather than the runtime state.
> > That leads to less cases and easier to understand semantics for the
> > migration stream.
>
> I am fine with this approach as well.
>
> >
> >> +}
> >> +
> >> +static const VMStateDescription vmstate_spapr_machine_check = {
> >> +.name = "spapr_machine_check",
> >> +.version_id = 1,
> >> +.minimum_version_id = 1,
> >> +.needed = spapr_fwnmi_needed,
> >> +.fields = (VMStateField[]) {
> >> +VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
> >> +VMSTATE_INT32(mc_status, SpaprMachineState),
> >> +VMSTATE_END_OF_LIST()
> >> +},
> >> +};
> >> +
> >> static const VMStateDescription vmstate_spapr = {
> >> .name = "spapr",
> >> .version_id = 3,
> >> @@ -2137,6 +2156,7 @@ static const VMStateDescription vmstate_spapr = {
> >> &vmstate_spapr_dtb,
> >> &vmstate_spapr_cap_large_decr,
> >> &vmstate_spapr_cap_ccf_assist,
> >> +&vmstate_spapr_machine_check,
> >> NULL
> >> }
> >> };
> >> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> >> index 573c0b7..35e21e4 100644
> >> --- a/hw/ppc/spapr_events.c
> >> +++ b/hw/ppc/spapr_events.c
> >> @@ -41,6 +41,7 @@
> >> #include "qemu/bcd.h"
> >> #include "hw/ppc/spapr_ovec.h"
> >> #include
> >> +#include "migration/blocker.h"
> >>
> >> #define RTAS_LOG_VERSION_MASK 0xff00
> >> #define RTAS_LOG_VERSION_60x0600
> >> @@ -855,6 +856,22 @@ static void spapr_mce_dispatch_elog(PowerPCCPU *cpu,
> >> bool recovered)
> >> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
> >> {
> >> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> >> +int ret;
> >> +Error *local_err = NULL;
> >> +
> >> +error_setg(&spapr->fwnmi_migration_blocker,
> >> +"Live migration not supported during machine check handling");
> >> +ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err);
> >> +if (ret < 0) {
> >> +/*
> >> + * We don't want to abort and let the migration to continue. In a
> >> + * rare case, the machine check handler will run on the target
> >> + * hardware. Though this is not preferable, it is better than
> >> aborting
> >> + * the migration or killing the VM.
> >> + */
> >> +error_free(spapr->fwnmi_migration_blocker);
> >
> > You should set fwnmi_migration_blocker to NULL here as well.
>
> ok.
>
> >
> > As mentioned on an earlier iteration, the migration blocker is the
> > same every time. Couldn't you just create it once and free at final
> > teardown, rather than recreating it for every NMI?
>
> That means, we create the error string at the time when ibm,nmi-register
> is invoked and tear it down during machine reset?
>
No, I think David is asking to create the error string during machine init,
for all the machine lifetime. In which case, we don't even need to call
error_free() at all.
> Regards,
> Aravinda
>
> >
> >> +warn_report_err(local_err);
> >> +}
> >>
> >> while (spapr->mc_status != -1) {
> >> /*
> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >> index 91a7ab9..c849223 100644
> >> --- a/hw/ppc/spapr_rtas.c
> >> +++ b/hw/ppc/spapr_rtas.c
> >> @@ -50,6 +50,7 @@
> >> #include "target/ppc/mmu-hash64.h"
> >> #include "target/ppc/mmu-book3s-v3.h"
> >> #include "kvm_ppc.h"
> >> +#include "migration/blocker.h"
> >>
> >> static void rtas_display_character(PowerPCCPU *cpu, Spapr
[Qemu-devel] [PATCH v2 04/10] target/mips: Unroll loops in helpers for MSA logic instructions
From: Aleksandar Markovic
Unroll loops in helpers for MSA logic instructions for better
performance.
Signed-off-by: Aleksandar Markovic
Reviewed-by: Aleksandar Rikalo
Message-Id: <1559745316-1454-4-git-send-email-aleksandar.marko...@rt-rk.com>
---
target/mips/msa_helper.c | 44
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 7b73e22..a861155 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -268,10 +268,6 @@ void helper_msa_ ## FUNC(CPUMIPSState *env, uint32_t wd,
uint32_t ws, \
} \
}
-MSA_FN_VECTOR(and_v, pwd->d[i], pws->d[i] & pwt->d[i])
-MSA_FN_VECTOR(or_v, pwd->d[i], pws->d[i] | pwt->d[i])
-MSA_FN_VECTOR(nor_v, pwd->d[i], ~(pws->d[i] | pwt->d[i]))
-MSA_FN_VECTOR(xor_v, pwd->d[i], pws->d[i] ^ pwt->d[i])
MSA_FN_VECTOR(bmnz_v, pwd->d[i],
BIT_MOVE_IF_NOT_ZERO(pwd->d[i], pws->d[i], pwt->d[i], DF_DOUBLE))
MSA_FN_VECTOR(bmz_v, pwd->d[i],
@@ -283,6 +279,46 @@ MSA_FN_VECTOR(bsel_v, pwd->d[i],
#undef BIT_SELECT
#undef MSA_FN_VECTOR
+void helper_msa_and_v(CPUMIPSState *env, uint32_t wd, uint32_t ws, uint32_t wt)
+{
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
+wr_t *pws = &(env->active_fpu.fpr[ws].wr);
+wr_t *pwt = &(env->active_fpu.fpr[wt].wr);
+
+pwd->d[0] = pws->d[0] & pwt->d[0];
+pwd->d[1] = pws->d[1] & pwt->d[1];
+}
+
+void helper_msa_or_v(CPUMIPSState *env, uint32_t wd, uint32_t ws, uint32_t wt)
+{
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
+wr_t *pws = &(env->active_fpu.fpr[ws].wr);
+wr_t *pwt = &(env->active_fpu.fpr[wt].wr);
+
+pwd->d[0] = pws->d[0] | pwt->d[0];
+pwd->d[1] = pws->d[1] | pwt->d[1];
+}
+
+void helper_msa_nor_v(CPUMIPSState *env, uint32_t wd, uint32_t ws, uint32_t wt)
+{
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
+wr_t *pws = &(env->active_fpu.fpr[ws].wr);
+wr_t *pwt = &(env->active_fpu.fpr[wt].wr);
+
+pwd->d[0] = ~(pws->d[0] | pwt->d[0]);
+pwd->d[1] = ~(pws->d[1] | pwt->d[1]);
+}
+
+void helper_msa_xor_v(CPUMIPSState *env, uint32_t wd, uint32_t ws, uint32_t wt)
+{
+wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
+wr_t *pws = &(env->active_fpu.fpr[ws].wr);
+wr_t *pwt = &(env->active_fpu.fpr[wt].wr);
+
+pwd->d[0] = pws->d[0] ^ pwt->d[0];
+pwd->d[1] = pws->d[1] ^ pwt->d[1];
+}
+
static inline int64_t msa_addv_df(uint32_t df, int64_t arg1, int64_t arg2)
{
return arg1 + arg2;
--
2.7.4
[Qemu-devel] [PATCH v2 00/10] Amend and clean up MSA support
From: Aleksandar Markovic This series contains various refactorings and cleanups of MSA support. Improve support for logic instructions, add tests for shift and FP max/min instructions, add reseting all MSA registers before running tests, improve code style, outline future organization. v1->v2: - tranfer "README" patch from another series to this one - add FP max/min tests - add reset_msa_registers() - move some tests to a better location Aleksandar Markovic (10): target/mips: Fix space-related format issues im msa_helper.c target/mips: Fix block-comment-related issues im msa_helper.c target/mips: Outline places for future MSA helpers target/mips: Unroll loops in helpers for MSA logic instructions tests/tcg: target/mips: Amend and rearrange MSA wrappers tests/tcg: target/mips: Add tests for MSA shift instructions tests/tcg: target/mips: Move four tests to a better location tests/tcg: target/mips: Add utility function reset_msa_registers() tests/tcg: target/mips: Add README for MSA tests tests/tcg: target/mips: Add tests for MSA FP max/min instructions target/mips/msa_helper.c | 252 ++-- tests/tcg/mips/include/wrappers_msa.h | 424 +- tests/tcg/mips/user/ase/msa/README | 639 + .../mips/user/ase/msa/bit-count/test_msa_nloc_b.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nloc_d.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nloc_h.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nloc_w.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nlzc_b.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nlzc_d.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nlzc_h.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_nlzc_w.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_pcnt_b.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_pcnt_d.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_pcnt_h.c | 2 + .../mips/user/ase/msa/bit-count/test_msa_pcnt_w.c | 2 + .../mips/user/ase/msa/bit-set/test_msa_bclr_b.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bclr_d.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bclr_h.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bclr_w.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bneg_b.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bneg_d.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bneg_h.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bneg_w.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bset_b.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bset_d.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bset_h.c| 2 + .../mips/user/ase/msa/bit-set/test_msa_bset_w.c| 2 + .../test_msa_mul_q_h.c | 2 + .../test_msa_mul_q_w.c | 2 + .../test_msa_mulr_q_h.c| 2 + .../test_msa_mulr_q_w.c| 2 + .../user/ase/msa/float-max-min/test_msa_fmax_a_d.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmax_a_w.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmax_d.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmax_w.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmin_a_d.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmin_a_w.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmin_d.c | 155 + .../user/ase/msa/float-max-min/test_msa_fmin_w.c | 155 + .../mips/user/ase/msa/int-add/test_msa_add_a_b.c | 2 + .../mips/user/ase/msa/int-add/test_msa_add_a_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_add_a_h.c | 2 + .../mips/user/ase/msa/int-add/test_msa_add_a_w.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_a_b.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_a_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_a_h.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_a_w.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_s_b.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_s_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_s_h.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_s_w.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_u_b.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_u_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_u_h.c | 2 + .../mips/user/ase/msa/int-add/test_msa_adds_u_w.c | 2 + .../mips/user/ase/msa/int-add/test_msa_addv_b.c| 2 + .../mips/user/ase/msa/int-add/test_msa_addv_d.c| 2 + .../mips/user/ase/msa/int-add/test_msa_addv_h.c| 2 + .../mips/user/ase/msa/int-add/test_msa_addv_w.c| 2 + .../mips/user/ase/msa/int-add/test_msa_hadd_s_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_hadd_s_h.c | 2 + .../mips/user/ase/msa/int-add/test_msa_hadd_s_w.c | 2 + .../mips/user/ase/msa/int-add/test_msa_hadd_u_d.c | 2 + .../mips/user/ase/msa/int-add/test_msa_hadd
[Qemu-devel] [PATCH v2 05/10] tests/tcg: target/mips: Amend and rearrange MSA wrappers
From: Aleksandar Markovic Amend and rearrange MSA wrappers to follow the same organization as in MSA tests. Signed-off-by: Aleksandar Markovic Reviewed-by: Aleksandar Rikalo Message-Id: <1559745316-1454-6-git-send-email-aleksandar.marko...@rt-rk.com> --- tests/tcg/mips/include/wrappers_msa.h | 418 +++--- 1 file changed, 279 insertions(+), 139 deletions(-) diff --git a/tests/tcg/mips/include/wrappers_msa.h b/tests/tcg/mips/include/wrappers_msa.h index 6feda36..2dc89e4 100644 --- a/tests/tcg/mips/include/wrappers_msa.h +++ b/tests/tcg/mips/include/wrappers_msa.h @@ -55,21 +55,6 @@ static inline void do_msa_##suffix(const void *input, \ ); \ } -DO_MSA__WD__WS(NLOC_B, nloc.b) -DO_MSA__WD__WS(NLOC_H, nloc.h) -DO_MSA__WD__WS(NLOC_W, nloc.w) -DO_MSA__WD__WS(NLOC_D, nloc.d) - -DO_MSA__WD__WS(NLZC_B, nlzc.b) -DO_MSA__WD__WS(NLZC_H, nlzc.h) -DO_MSA__WD__WS(NLZC_W, nlzc.w) -DO_MSA__WD__WS(NLZC_D, nlzc.d) - -DO_MSA__WD__WS(PCNT_B, pcnt.b) -DO_MSA__WD__WS(PCNT_H, pcnt.h) -DO_MSA__WD__WS(PCNT_W, pcnt.w) -DO_MSA__WD__WS(PCNT_D, pcnt.d) - #define DO_MSA__WD__WS_WT(suffix, mnemonic)\ static inline void do_msa_##suffix(const void *input1, \ @@ -126,85 +111,52 @@ static inline void do_msa_##suffix(void *input1, void *input2, \ ); \ } -DO_MSA__WD__WS_WT(ILVEV_B, ilvev.b) -DO_MSA__WD__WS_WT(ILVEV_H, ilvev.h) -DO_MSA__WD__WS_WT(ILVEV_W, ilvev.w) -DO_MSA__WD__WS_WT(ILVEV_D, ilvev.d) -DO_MSA__WD__WS_WT(ILVOD_B, ilvod.b) -DO_MSA__WD__WS_WT(ILVOD_H, ilvod.h) -DO_MSA__WD__WS_WT(ILVOD_W, ilvod.w) -DO_MSA__WD__WS_WT(ILVOD_D, ilvod.d) - -DO_MSA__WD__WS_WT(ILVL_B, ilvl.b) -DO_MSA__WD__WS_WT(ILVL_H, ilvl.h) -DO_MSA__WD__WS_WT(ILVL_W, ilvl.w) -DO_MSA__WD__WS_WT(ILVL_D, ilvl.d) - -DO_MSA__WD__WS_WT(ILVR_B, ilvr.b) -DO_MSA__WD__WS_WT(ILVR_H, ilvr.h) -DO_MSA__WD__WS_WT(ILVR_W, ilvr.w) -DO_MSA__WD__WS_WT(ILVR_D, ilvr.d) - -DO_MSA__WD__WS_WT(AND_V, and.v) -DO_MSA__WD__WS_WT(NOR_V, nor.v) -DO_MSA__WD__WS_WT(OR_V, or.v) -DO_MSA__WD__WS_WT(XOR_V, xor.v) - -DO_MSA__WD__WS_WT(CEQ_B, ceq.b) -DO_MSA__WD__WS_WT(CEQ_H, ceq.h) -DO_MSA__WD__WS_WT(CEQ_W, ceq.w) -DO_MSA__WD__WS_WT(CEQ_D, ceq.d) +/* + * Bit Count + * - + */ -DO_MSA__WD__WS_WT(CLE_S_B, cle_s.b) -DO_MSA__WD__WS_WT(CLE_S_H, cle_s.h) -DO_MSA__WD__WS_WT(CLE_S_W, cle_s.w) -DO_MSA__WD__WS_WT(CLE_S_D, cle_s.d) +DO_MSA__WD__WS(NLOC_B, nloc.b) +DO_MSA__WD__WS(NLOC_H, nloc.h) +DO_MSA__WD__WS(NLOC_W, nloc.w) +DO_MSA__WD__WS(NLOC_D, nloc.d) -DO_MSA__WD__WS_WT(CLE_U_B, cle_u.b) -DO_MSA__WD__WS_WT(CLE_U_H, cle_u.h) -DO_MSA__WD__WS_WT(CLE_U_W, cle_u.w) -DO_MSA__WD__WS_WT(CLE_U_D, cle_u.d) +DO_MSA__WD__WS(NLZC_B, nlzc.b) +DO_MSA__WD__WS(NLZC_H, nlzc.h) +DO_MSA__WD__WS(NLZC_W, nlzc.w) +DO_MSA__WD__WS(NLZC_D, nlzc.d) -DO_MSA__WD__WS_WT(CLT_S_B, clt_s.b) -DO_MSA__WD__WS_WT(CLT_S_H, clt_s.h) -DO_MSA__WD__WS_WT(CLT_S_W, clt_s.w) -DO_MSA__WD__WS_WT(CLT_S_D, clt_s.d) +DO_MSA__WD__WS(PCNT_B, pcnt.b) +DO_MSA__WD__WS(PCNT_H, pcnt.h) +DO_MSA__WD__WS(PCNT_W, pcnt.w) +DO_MSA__WD__WS(PCNT_D, pcnt.d) -DO_MSA__WD__WS_WT(CLT_U_B, clt_u.b) -DO_MSA__WD__WS_WT(CLT_U_H, clt_u.h) -DO_MSA__WD__WS_WT(CLT_U_W, clt_u.w) -DO_MSA__WD__WS_WT(CLT_U_D, clt_u.d) -DO_MSA__WD__WS_WT(MAX_A_B, max_a.b) -DO_MSA__WD__WS_WT(MAX_A_H, max_a.h) -DO_MSA__WD__WS_WT(MAX_A_W, max_a.w) -DO_MSA__WD__WS_WT(MAX_A_D, max_a.d) +/* + * Bit move + * + */ -DO_MSA__WD__WS_WT(MIN_A_B, min_a.b) -DO_MSA__WD__WS_WT(MIN_A_H, min_a.h) -DO_MSA__WD__WS_WT(MIN_A_W, min_a.w) -DO_MSA__WD__WS_WT(MIN_A_D, min_a.d) +DO_MSA__WD__WS(BINSL_B, binsl.b) +DO_MSA__WD__WS(BINSL_H, binsl.h) +DO_MSA__WD__WS(BINSL_W, binsl.w) +DO_MSA__WD__WS(BINSL_D, binsl.d) -DO_MSA__WD__WS_WT(MAX_S_B, max_s.b) -DO_MSA__WD__WS_WT(MAX_S_H, max_s.h) -DO_MSA__WD__WS_WT(MAX_S_W, max_s.w) -DO_MSA__WD__WS_WT(MAX_S_D, max_s.d) +DO_MSA__WD__WS(BINSR_B, binsr.b) +DO_MSA__WD__WS(BINSR_H, binsr.h) +DO_MSA__WD__WS(BINSR_W, binsr.w) +DO_MSA__WD__WS(BINSR_D, binsr.d) -DO_MSA__WD__WS_WT(MIN_S_B, min_s.b) -DO_MSA__WD__WS_WT(MIN_S_H, min_s.h) -DO_MSA__WD__WS_WT(MIN_S_W, min_s.w) -DO_MSA__WD__WS_WT(MIN_S_D, min_s.d) +DO_MSA__WD__WS_WT(BMZ_V, bmz.v) +DO_MSA__WD__WS_WT(BMNZ_V, bmnz.v) +DO_MSA__WD__WS_WT(BSEL_V, bsel.v) -DO_MSA__WD__WS_WT(MAX_U_B, max_u.b) -DO_MSA__WD__WS_WT(MAX_U_H, max_u.h) -DO_MSA__WD__WS_WT(MAX_U_W, max_u.w) -DO_MSA__WD__WS_WT(MAX_U_D, max_u.d) -DO_MSA__WD__WS_WT(MIN_U_B, min_u.b) -DO_MSA__WD__WS_WT(MIN_U_H, min_u.h) -DO_MSA__WD__WS_WT(MIN_U_W, min_u.w) -DO_MSA__WD__WS_WT(MIN_U_D, min_u.d) +/* + * Bit Set + * --- + */ DO_MSA__WD__WS_WT(BCLR_B, bclr.b) DO_MSA__WD__WS_WT(BCLR_H, bclr.h) @@ -221,45 +173,53 @@ DO_MSA__WD__WS_WT(BNEG_H, bneg.h) DO_MSA__WD__WS_WT(BNEG_W, bneg.w) DO_MSA__WD__WS_WT(BNEG_D, bneg.d) -DO_MSA__WD__WS_WT(PCKEV_B, pckev.b) -DO_MSA__WD__WS_WT(PCKEV_H, pckev.h) -DO_MSA__WD__WS_WT(PCK
[Qemu-devel] [PATCH v2 03/10] target/mips: Outline places for future MSA helpers
From: Aleksandar Markovic
Outline places for future MSA helpers to follow the same organization
as in MSA tests.
Signed-off-by: Aleksandar Markovic
Reviewed-by: Aleksandar Rikalo
Message-Id: <1559745316-1454-7-git-send-email-aleksandar.marko...@rt-rk.com>
---
target/mips/msa_helper.c | 138 +++
1 file changed, 138 insertions(+)
diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index 851450c..7b73e22 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -42,6 +42,144 @@
/* Element-by-element access macros */
#define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df))
+
+
+/*
+ * Bit Count
+ * -
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Bit move
+ *
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Bit Set
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Fixed Multiply
+ * --
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Add
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Average
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Compare
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Divide
+ * --
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Dot Product
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Max Min
+ * ---
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Modulo
+ * --
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Multiply
+ *
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Int Subtract
+ *
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Interleave
+ * --
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Logic
+ * -
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Pack
+ *
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
+/*
+ * Shift
+ * -
+ */
+
+/* TODO: insert appropriate helpers here */
+
+
static inline void msa_move_v(wr_t *pwd, wr_t *pws)
{
uint32_t i;
--
2.7.4
[Qemu-devel] [PATCH v2 02/10] target/mips: Fix block-comment-related issues im msa_helper.c
From: Aleksandar Markovic
Fix block-comment-related issues reported by checkpatch for file
msa_helper.c.
Signed-off-by: Aleksandar Markovic
Reviewed-by: Aleksandar Rikalo
Message-Id: <1559745316-1454-3-git-send-email-aleksandar.marko...@rt-rk.com>
---
target/mips/msa_helper.c | 42 +++---
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index eacb5a4..851450c 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -1975,8 +1975,10 @@ static inline int update_msacsr(CPUMIPSState *env, int
action, int denormal)
c &= ~FP_UNDERFLOW;
}
-/* Reciprocal operations set only Inexact when valid and not
- divide by zero */
+/*
+ * Reciprocal operations set only Inexact when valid and not
+ * divide by zero
+ */
if ((action & RECIPROCAL_INEXACT) &&
(c & (FP_INVALID | FP_DIV0)) == 0) {
c = FP_INEXACT;
@@ -1985,15 +1987,19 @@ static inline int update_msacsr(CPUMIPSState *env, int
action, int denormal)
cause = c & enable;/* all current enabled exceptions */
if (cause == 0) {
-/* No enabled exception, update the MSACSR Cause
- with all current exceptions */
+/*
+ * No enabled exception, update the MSACSR Cause
+ * with all current exceptions
+ */
SET_FP_CAUSE(env->active_tc.msacsr,
(GET_FP_CAUSE(env->active_tc.msacsr) | c));
} else {
/* Current exceptions are enabled */
if ((env->active_tc.msacsr & MSACSR_NX_MASK) == 0) {
-/* Exception(s) will trap, update MSACSR Cause
- with all enabled exceptions */
+/*
+ * Exception(s) will trap, update MSACSR Cause
+ * with all enabled exceptions
+ */
SET_FP_CAUSE(env->active_tc.msacsr,
(GET_FP_CAUSE(env->active_tc.msacsr) | c));
}
@@ -3059,9 +3065,11 @@ void helper_msa_fexdo_df(CPUMIPSState *env, uint32_t df,
uint32_t wd,
switch (df) {
case DF_WORD:
for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) {
-/* Half precision floats come in two formats: standard
- IEEE and "ARM" format. The latter gains extra exponent
- range by omitting the NaN/Inf encodings. */
+/*
+ * Half precision floats come in two formats: standard
+ * IEEE and "ARM" format. The latter gains extra exponent
+ * range by omitting the NaN/Inf encodings.
+ */
flag ieee = 1;
MSA_FLOAT_BINOP(Lh(pwx, i), from_float32, pws->w[i], ieee, 16);
@@ -3617,9 +3625,11 @@ void helper_msa_fexupl_df(CPUMIPSState *env, uint32_t
df, uint32_t wd,
switch (df) {
case DF_WORD:
for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) {
-/* Half precision floats come in two formats: standard
- IEEE and "ARM" format. The latter gains extra exponent
- range by omitting the NaN/Inf encodings. */
+/*
+ * Half precision floats come in two formats: standard
+ * IEEE and "ARM" format. The latter gains extra exponent
+ * range by omitting the NaN/Inf encodings.
+ */
flag ieee = 1;
MSA_FLOAT_BINOP(pwx->w[i], from_float16, Lh(pws, i), ieee, 32);
@@ -3651,9 +3661,11 @@ void helper_msa_fexupr_df(CPUMIPSState *env, uint32_t
df, uint32_t wd,
switch (df) {
case DF_WORD:
for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) {
-/* Half precision floats come in two formats: standard
- IEEE and "ARM" format. The latter gains extra exponent
- range by omitting the NaN/Inf encodings. */
+/*
+ * Half precision floats come in two formats: standard
+ * IEEE and "ARM" format. The latter gains extra exponent
+ * range by omitting the NaN/Inf encodings.
+ */
flag ieee = 1;
MSA_FLOAT_BINOP(pwx->w[i], from_float16, Rh(pws, i), ieee, 32);
--
2.7.4
