Re: [PATCH v4 5/9] target/arm/kvm64: Add kvm_arch_get/put_sve
On Tue, Sep 24, 2019 at 01:31:01PM +0200, Andrew Jones wrote:
> These are the SVE equivalents to kvm_arch_get/put_fpsimd. Note, the
> swabbing is different than it is for fpsmid because the vector format
> is a little-endian stream of words.
>
> Signed-off-by: Andrew Jones
> Reviewed-by: Richard Henderson
> ---
> target/arm/kvm64.c | 137 +++--
> 1 file changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 28f6db57d5ee..ea454c613919 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -671,11 +671,12 @@ int kvm_arch_destroy_vcpu(CPUState *cs)
> bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
> {
> /* Return true if the regidx is a register we should synchronize
> - * via the cpreg_tuples array (ie is not a core reg we sync by
> - * hand in kvm_arch_get/put_registers())
> + * via the cpreg_tuples array (ie is not a core or sve reg that
> + * we sync by hand in kvm_arch_get/put_registers())
> */
> switch (regidx & KVM_REG_ARM_COPROC_MASK) {
> case KVM_REG_ARM_CORE:
> +case KVM_REG_ARM64_SVE:
> return false;
> default:
> return true;
> @@ -761,6 +762,78 @@ static int kvm_arch_put_fpsimd(CPUState *cs)
> return 0;
> }
>
> +/*
> + * SVE registers are encoded in KVM's memory in an endianness-invariant
> format.
> + * The byte at offset i from the start of the in-memory representation
> contains
> + * the bits [(7 + 8 * i) : (8 * i)] of the register value. As this means the
> + * lowest offsets are stored in the lowest memory addresses, then that nearly
> + * matches QEMU's representation, which is to use an array of host-endian
> + * uint64_t's, where the lower offsets are at the lower indices. To complete
> + * the translation we just need to byte swap the uint64_t's on big-endian
> hosts.
> + */
> +static uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
> +{
> +#ifdef HOST_WORDS_BIGENDIAN
> +int i;
> +
> +for (i = 0; i < nr; ++i) {
> +dst[i] = bswap64(src[i]);
> +}
> +
> +return dst;
> +#else
> +return src;
> +#endif
> +}
> +
> +/*
> + * KVM SVE registers come in slices where ZREGs have a slice size of 2048
> bits
> + * and PREGS and the FFR have a slice size of 256 bits. However we simply
> hard
> + * code the slice index to zero for now as it's unlikely we'll need more than
> + * one slice for quite some time.
> + */
> +static int kvm_arch_put_sve(CPUState *cs)
> +{
> +ARMCPU *cpu = ARM_CPU(cs);
> +CPUARMState *env = &cpu->env;
> +uint64_t tmp[ARM_MAX_VQ * 2];
> +uint64_t *r;
> +struct kvm_one_reg reg;
> +int n, ret;
> +
> +for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
> +r = sve_bswap64(tmp, &env->vfp.zregs[n].d[0], cpu->sve_max_vq * 2);
> +reg.addr = (uintptr_t)r;
> +reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
> +ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
> +if (ret) {
> +return ret;
> +}
> +}
> +
> +for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
> +r = sve_bswap64(tmp, r = &env->vfp.pregs[n].p[0],
> +DIV_ROUND_UP(cpu->sve_max_vq, 8));
I see a bug here that I introduced between v2 and v3 when I switched to
DIV_ROUND_UP. I dropped the '* 2's on all of these. They should be
DIV_ROUND_UP(cpu->sve_max_vq * 2, 8). I'll fix for v5, which I'll be
posting later today.
Thanks,
drew
Re: [PATCH v2 21/33] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController
On Tue, Oct 01, 2019 at 07:43:51AM +0200, Cédric Le Goater wrote:
> On 01/10/2019 04:31, David Gibson wrote:
> > On Mon, Sep 30, 2019 at 12:13:14PM +0200, Cédric Le Goater wrote:
> >> On 30/09/2019 08:14, David Gibson wrote:
> >>> On Mon, Sep 30, 2019 at 07:28:45AM +0200, Cédric Le Goater wrote:
> On 30/09/2019 03:49, David Gibson wrote:
> > On Fri, Sep 27, 2019 at 12:16:49PM +0200, Greg Kurz wrote:
> >> On Fri, 27 Sep 2019 15:50:16 +1000
> >> David Gibson wrote:
> >>
> >>> This method essentially represents code which belongs to the interrupt
> >>> controller, but needs to be called on all possible intcs, rather than
> >>> just the currently active one. The "dual" version therefore calls
> >>> into the xics and xive versions confusingly.
> >>>
> >>> Handle this more directly, by making it instead a method on the intc
> >>> backend, and always calling it on every backend that exists.
> >>>
> >>> While we're there, streamline the error reporting a bit.
> >>>
> >>> Signed-off-by: David Gibson
> > [snip]
> >>> @@ -525,6 +469,30 @@ static void spapr_irq_check(SpaprMachineState
> >>> *spapr, Error **errp)
> >>> /*
> >>> * sPAPR IRQ frontend routines for devices
> >>> */
> >>> +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr,
> >>> + PowerPCCPU *cpu, Error **errp)
> >>> +{
> >>> +if (spapr->xive) {
> >>> +SpaprInterruptController *intc = SPAPR_INTC(spapr->xive);
> >>> +SpaprInterruptControllerClass *sicc =
> >>> SPAPR_INTC_GET_CLASS(intc);
> >>> +
> >>> +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
> >>> +return -1;
> >>> +}
> >>> +}
> >>> +
> >>> +if (spapr->ics) {
> >>> +SpaprInterruptController *intc = SPAPR_INTC(spapr->ics);
> >>> +SpaprInterruptControllerClass *sicc =
> >>> SPAPR_INTC_GET_CLASS(intc);
> >>> +
> >>> +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
> >>> +return -1;
> >>> +}
> >>> +}
> >>> +
> >>
> >> Instead of these hooks, what about open-coding
> >> spapr_xive_cpu_intc_create()
> >> and xics_spapr_cpu_intc_create() directly here, like you already did
> >> for the
> >> ICS and the XIVE objects in spapr_irq_init() ?
> >
> > I'd prefer not to. The idea is I want to treat this as basically:
> >
> > foreach_possible_intc(intc)
> > intc::cpu_intc_create(...)
> >
> > If I find time I might indeed replace the explicit ics and xive
> > pointers with just an array of SpaprInterruptController *.
>
> Or you could use object_child_foreach() and check for the type. If we had
> a helper object_child_foreach_type(), we could use it elsewhere.
> >>>
> >>> I thought about that, but I don't think it quite works. The
> >>> complication is that the xics device is made explicitly a child of the
> >>> machine, but the xive device has mmio, so it's a SusBusDevice sitting
> >>> on the root bus instead.
> >>
> >> PnvXscom works fine with Devices and SysBusDevices.
> >
> > Uh... what's an example of it working with a SysBusDevice? All the
> > implementors of PNV_XSCOM_INTERFACE I could find were instantiated
> > with object_initialize_child() making them explicitly children of the
> > chip. The SPAPR_XIVE is instantiated with qdev_create(NULL,
> > TYPE_SPAPR_XIVE), making it a child of the root bus, not the machine,
> > I believe.
>
> I see. We should reparent the interrupt controller then.
Well, maybe. It's not obvious to me that that's the right approach
just because of this.
> Could we rework
> the code to instantiate and realize the XICS and XIVE model objects ?
> We have the handlers spapr_instance_init() and spapr_machine_init().
I'm not really sure what you're suggesting here.
> That always has been a problem IMO.
>
>
> C.
>
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 1/2] qapi: Add feature flags to commands in qapi introspection
Peter Krempa writes: > Similarly to features for struct types introduce the feature flags also > for commands. This will allow notifying management layers of fixes and > compatible changes in the behaviour of a command which may not be > detectable any other way. > > The changes were heavily inspired by commit 6a8c0b51025. > > Signed-off-by: Peter Krempa +1 on adding features to commands. Patch conflicts with the technical debt payback work I posted before and after this series, although not nearly as badly as I expected. It'll conflict some more with the parts I haven't flushed. The funny bit: I went on that rampage in preparation of QAPI language extensions including "features everywhere, not just structs". I'll look into how to best fit your work into mine.
Re: [PATCH v4 15/24] audio: add mixing-engine option (documentation)
"Zoltán Kővágó" writes: > On 2019-09-25 11:49, Markus Armbruster wrote: >> "Zoltán Kővágó" writes: >> >>> On 2019-09-23 15:08, Markus Armbruster wrote: "Kővágó, Zoltán" writes: > This will allow us to disable mixeng when we use a decent backend. > > Disabling mixeng have a few advantages: > * we no longer convert the audio output from one format to another, when > the underlying audio system would just convert it to a third format. > We no longer convert, only the underlying system, when needed. > * the underlying system probably has better resampling and sample format > converting methods anyway... > * we may support formats that the mixeng currently does not support (S24 > or float samples, more than two channels) > * when using an audio server (like pulseaudio) different sound card > outputs will show up as separate streams, even if we use only one > backend > > Disadvantages: > * audio capturing no longer works (wavcapture, and vnc audio extension) > * some backends only support a single playback stream or very picky > about the audio format. In this case we can't disable mixeng. > > However mixeng is not removed, only made optional, so this shouldn't be > a big concern. > > Signed-off-by: Kővágó, Zoltán > --- > > Notes: > Changes from v1: >* renamed mixeng to mixing-engine > >qapi/audio.json | 5 + >qemu-options.hx | 6 ++ >2 files changed, 11 insertions(+) > > diff --git a/qapi/audio.json b/qapi/audio.json > index 9fefdf5186..0535eff794 100644 > --- a/qapi/audio.json > +++ b/qapi/audio.json > @@ -11,6 +11,10 @@ ># General audio backend options that are used for both playback and ># recording. ># > +# @mixing-engine: use QEMU's mixing engine to mix all streams inside > QEMU. When > +# set to off, fixed-settings must be also off. Not every > backend > +# compatible with the off setting (default on, since 4.2) > +# Last sentence no verb. Which backends are compatible? >>> >>> Actually that's a simplification, it depends on a few things. When >>> mixeng is off, qemu will try to use the same format as the emulated >>> sound card, and if the backend doesn't support that format, it won't >>> work (no audio). Also attaching multiple sound cards to the same >>> audiodev might not work, if the backend doesn't support multiple >>> playback streams. If you use pulseaudio, it'll work without problems, >>> if you use alsa, it depends on your device. If you use a hw: device >>> directly, you'll likely only be able to use one emulated sound card >>> with a few selected audio formats. If you use dmix: (and plug), alsa >>> will handle the conversion and mixing, so it will work no matter what >>> format the emulated sound card uses. With OSS the situation is >>> probably similar, it depends on the kernel/hw what works and what not. >>> wav and spice certainly doesn't support multiple streams. I'm not >>> completely sure about the other backends right now, but I think dsound >>> and coreaudio can handle the necessary sample format conversions and >>> mixing. >>> What happens when you try the off setting with incompatible backends? >>> See above. >> >> What happens *exactly*? >> >> I'm asking because I'm concerned about the user experience. When a user >> asks for a combination of things QEMU can't provide, such as mixeng off >> with an incompatible backend, QEMU should fail with a suitable error >> message. Does it? > > Error handling is not the best in the audio subsystem, if something > fails it generally just prints a warning to the console and continues, > and something will happen... For example, this is what happens when I > try to open one hw device twice. I ran qemu with: > > -audiodev > alsa,id=foo,in.dev=hw:1,,0,out.mixing-engine=off,out.dev=hw:1,,0 > -device piix4-usb-uhci -device usb-audio,audiodev=foo -device > AC97,audiodev=foo > > When the guest tried to initialize the AC97 card, I got an error: > > alsa: Could not initialize DAC > alsa: Failed to open `hw:1,0': > alsa: Reason: Device or resource busy > > And it just continued. And the sound worked, but with wrong sample > rate (AC97 wants 44100 Hz, but USB audio previously opened the alsa > device with 48000 Hz). I'll fix this bug in the next revision, > audio_pcm_hw_add_* shouldn't fall back to other HWs without mixeng. > But even with that, the result will be that one emulated sound card > will work and the other won't. > > It's not ideal, but fixing it would require a lot of effort. Right > now, if you specify an invalid audiodev for alsa (even with mixeng), > it'll just print an error to the console and continue without audio. Should we document this general error handling deficiency somehow? >> Sometime
Re: [PATCH v2 21/33] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController
On 01/10/2019 04:31, David Gibson wrote:
> On Mon, Sep 30, 2019 at 12:13:14PM +0200, Cédric Le Goater wrote:
>> On 30/09/2019 08:14, David Gibson wrote:
>>> On Mon, Sep 30, 2019 at 07:28:45AM +0200, Cédric Le Goater wrote:
On 30/09/2019 03:49, David Gibson wrote:
> On Fri, Sep 27, 2019 at 12:16:49PM +0200, Greg Kurz wrote:
>> On Fri, 27 Sep 2019 15:50:16 +1000
>> David Gibson wrote:
>>
>>> This method essentially represents code which belongs to the interrupt
>>> controller, but needs to be called on all possible intcs, rather than
>>> just the currently active one. The "dual" version therefore calls
>>> into the xics and xive versions confusingly.
>>>
>>> Handle this more directly, by making it instead a method on the intc
>>> backend, and always calling it on every backend that exists.
>>>
>>> While we're there, streamline the error reporting a bit.
>>>
>>> Signed-off-by: David Gibson
> [snip]
>>> @@ -525,6 +469,30 @@ static void spapr_irq_check(SpaprMachineState
>>> *spapr, Error **errp)
>>> /*
>>> * sPAPR IRQ frontend routines for devices
>>> */
>>> +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr,
>>> + PowerPCCPU *cpu, Error **errp)
>>> +{
>>> +if (spapr->xive) {
>>> +SpaprInterruptController *intc = SPAPR_INTC(spapr->xive);
>>> +SpaprInterruptControllerClass *sicc =
>>> SPAPR_INTC_GET_CLASS(intc);
>>> +
>>> +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
>>> +return -1;
>>> +}
>>> +}
>>> +
>>> +if (spapr->ics) {
>>> +SpaprInterruptController *intc = SPAPR_INTC(spapr->ics);
>>> +SpaprInterruptControllerClass *sicc =
>>> SPAPR_INTC_GET_CLASS(intc);
>>> +
>>> +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
>>> +return -1;
>>> +}
>>> +}
>>> +
>>
>> Instead of these hooks, what about open-coding
>> spapr_xive_cpu_intc_create()
>> and xics_spapr_cpu_intc_create() directly here, like you already did for
>> the
>> ICS and the XIVE objects in spapr_irq_init() ?
>
> I'd prefer not to. The idea is I want to treat this as basically:
>
> foreach_possible_intc(intc)
> intc::cpu_intc_create(...)
>
> If I find time I might indeed replace the explicit ics and xive
> pointers with just an array of SpaprInterruptController *.
Or you could use object_child_foreach() and check for the type. If we had
a helper object_child_foreach_type(), we could use it elsewhere.
>>>
>>> I thought about that, but I don't think it quite works. The
>>> complication is that the xics device is made explicitly a child of the
>>> machine, but the xive device has mmio, so it's a SusBusDevice sitting
>>> on the root bus instead.
>>
>> PnvXscom works fine with Devices and SysBusDevices.
>
> Uh... what's an example of it working with a SysBusDevice? All the
> implementors of PNV_XSCOM_INTERFACE I could find were instantiated
> with object_initialize_child() making them explicitly children of the
> chip. The SPAPR_XIVE is instantiated with qdev_create(NULL,
> TYPE_SPAPR_XIVE), making it a child of the root bus, not the machine,
> I believe.
I see. We should reparent the interrupt controller then, Could we rework
the code to instantiate and realize the XICS and XIVE model objects ?
We have the handlers spapr_instance_init() and spapr_machine_init().
That always has been a problem IMO.
C.
Re: [PATCH v3 6/7] s390x/mmu: DAT table definition overhaul
On 27/09/2019 11.58, David Hildenbrand wrote: > Let's use consitent names for the region/section/page table entries and > for the macros to extract relevant parts from virtual address. Make them > match the definitions in the PoP - e.g., how the relevant bits are actually > called. > > Introduce defines for all bits declared in the PoP. This will come in > handy in follow-up patches. > > Add a note where additional information about s390x and the used > definitions can be found. > > Signed-off-by: David Hildenbrand > --- > target/s390x/cpu.h| 81 +-- > target/s390x/mem_helper.c | 12 +++--- > target/s390x/mmu_helper.c | 37 ++ > 3 files changed, 87 insertions(+), 43 deletions(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 163dae13d7..690b94c8ea 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -1,6 +1,10 @@ > /* > * S/390 virtual CPU header > * > + * For details on the s390x architecture and used definitions (e.g., > + * PSW, PER and DAT (Dynamic Address Translation)), please refer to > + * the "z/Architecture Principles of Operations" - a.k.a. PoP. > + * > * Copyright (c) 2009 Ulrich Hecht > * Copyright IBM Corp. 2012, 2018 > * > @@ -558,26 +562,63 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096); > #define ASCE_TYPE_SEGMENT 0x00/* segment table type > */ > #define ASCE_TABLE_LENGTH 0x03/* region table length > */ > > -#define REGION_ENTRY_ORIGIN (~0xfffULL) /* region/segment table origin > */ > -#define REGION_ENTRY_RO 0x200 /* region/segment protection bit > */ > -#define REGION_ENTRY_TF 0xc0/* region/segment table offset > */ > -#define REGION_ENTRY_INV 0x20/* invalid region table entry > */ > -#define REGION_ENTRY_TYPE_MASK 0x0c /* region/segment table type mask > */ > -#define REGION_ENTRY_TYPE_R1 0x0c/* region first table type > */ > -#define REGION_ENTRY_TYPE_R2 0x08/* region second table type > */ > -#define REGION_ENTRY_TYPE_R3 0x04/* region third table type > */ > -#define REGION_ENTRY_LENGTH 0x03/* region third length > */ > - > -#define SEGMENT_ENTRY_ORIGIN (~0x7ffULL) /* segment table origin*/ > -#define SEGMENT_ENTRY_FC 0x400 /* format control */ > -#define SEGMENT_ENTRY_RO 0x200 /* page protection bit */ > -#define SEGMENT_ENTRY_INV 0x20/* invalid segment table entry */ > - > -#define VADDR_PX 0xff000 /* page index bits */ > - > -#define PAGE_RO 0x200 /* HW read-only bit */ > -#define PAGE_INVALID 0x400 /* HW invalid bit*/ > -#define PAGE_RES0 0x800 /* bit must be zero */ > +#define REGION_ENTRY_ORIGIN 0xf000ULL > +#define REGION_ENTRY_P 0x0200ULL > +#define REGION_ENTRY_TF 0x00c0ULL > +#define REGION_ENTRY_I 0x0020ULL > +#define REGION_ENTRY_TT 0x000cULL > +#define REGION_ENTRY_TL 0x0003ULL > + > +#define REGION_ENTRY_TT_REGION1 0x000cULL > +#define REGION_ENTRY_TT_REGION2 0x0008ULL > +#define REGION_ENTRY_TT_REGION3 0x0004ULL > + > +#define REGION3_ENTRY_RFAA 0x8000ULL > +#define REGION3_ENTRY_AV0x0001ULL > +#define REGION3_ENTRY_ACC 0xf000ULL > +#define REGION3_ENTRY_F 0x0800ULL > +#define REGION3_ENTRY_FC0x0400ULL > +#define REGION3_ENTRY_IEP 0x0100ULL > +#define REGION3_ENTRY_CR0x0010ULL > + > +#define SEGMENT_ENTRY_ORIGIN0xf800ULL > +#define SEGMENT_ENTRY_SFAA 0xfff8ULL I think SFAA should be 0xfff0ULL instead? > +#define SEGMENT_ENTRY_AV0x0001ULL > +#define SEGMENT_ENTRY_ACC 0xf000ULL > +#define SEGMENT_ENTRY_F 0x0800ULL > +#define SEGMENT_ENTRY_FC0x0400ULL > +#define SEGMENT_ENTRY_P 0x0200ULL > +#define SEGMENT_ENTRY_IEP 0x0100ULL > +#define SEGMENT_ENTRY_I 0x0020ULL > +#define SEGMENT_ENTRY_CS0x0010ULL > +#define SEGMENT_ENTRY_TT0x000cULL > + > +#define SEGMENT_ENTRY_TT_REGION10x000cULL > +#define SEGMENT_ENTRY_TT_REGION20x0008ULL > +#define SEGMENT_ENTRY_TT_REGION30x0004ULL The above three definitions do not make much sense. TT should always be 00 for segment table entries. > +#define SEGMENT_ENTRY_TT_SEGMENT0xULL > + > +#define PAGE_ENTRY_00x000
Re: bitmap migration bug with -drive while block mirror runs
On Mon, Sep 30, 2019 at 20:09:28 -0400, John Snow wrote: > Hi folks, I identified a problem with the migration code that Red Hat QE > found and thought you'd like to see it: > > https://bugzilla.redhat.com/show_bug.cgi?id=1652424#c20 > > Very, very briefly: drive-mirror inserts a filter node that changes what > bdrv_get_device_or_node_name() returns, which causes a migration problem. > > Ignorant question #1: Can we multi-parent the filter node and > source-node? It looks like at the moment both consider their only parent > to be the block-job and don't have a link back to their parents otherwise. > > > Otherwise: I have a lot of cloudy ideas on how to solve this, but > ultimately what we want is to be able to find the "addressable" name for > the node the bitmap is attached to, which would be the name of the first > ancestor node that isn't a filter. (OR, the name of the block-backend > above that node.) One possibility if there isn't an elegant qemu-based solution would be to add a migration feature libvirt could enable which would simply stop bitmaps from being copied and libvirt would do that in the synchronised phase of the migration explicitly. Libvirt might possibly need to do it anyways for inactive bitmaps if the automatic bitmap copying includes only active bitmaps. I'm not sure though how that would combine with post-copy migration or what the impact on latency would be, but if you are migrating with storage I think performance will not be stelar anyways. > A simple way to do this might be a "child_unfiltered" BdrvChild role > that simply bypasses the filter that was inserted and serves no real > purpose other than to allow the child to have a parent link and find who > it's """real""" parent is. > > Because of flushing, reopen, sync, drain &c &c &c I'm not sure how > feasible this quick idea might be, though. > > > - Corollary fix #1: call error_setg if the bitmap node name that's about > to go over the wire is an autogenerated node: this is never correct! > > (Why not? because the target is incapable of matching the node-name > because they are randomly generated AND you cannot specify node-names > with # prefixes as they are especially reserved! > > (This raises a related problem: if you explicitly add bitmaps to nodes > with autogenerated names, you will be unable to migrate them.)) I think this should be okay. In libvirt I opted to forbid checkpoints which map to bitmap creation until blockdev will be supported where we manage node names ourselves.
[Bug 1623998] Re: pulseaudio Invalid argument error
Triaging old bug tickets ... Can you still reproduce this issue with the latest version of QEMU (currently 4.1)? Or could we close this ticket nowadays? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1623998 Title: pulseaudio Invalid argument error Status in QEMU: Incomplete Bug description: When using qemu-system-ppc on Ubuntu Mate 15 with the usb audio card, I see these error messages: pulseaudio: set_sink_input_volume() failed pulseaudio: Reason: Invalid argument pulseaudio: set_sink_input_mute() failed pulseaudio: Reason: Invalid argument No audio plays. When an attempt is made, QEMU seems to freeze for a moment. I use "-device usb-audio" to add the usb sound card. This issue is present in both emulation and KVM mode. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1623998/+subscriptions
[Bug 1618265] Re: Loading virtio-input-host-pci drivers before boot? To allow using passthrough devices in grub and other preboot menus?
This can not be fixed on the QEMU side. If you want to have virtio-input support in seabios or grub for example, you've got to ask the seabios or grub project to add it. ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1618265 Title: Loading virtio-input-host-pci drivers before boot? To allow using passthrough devices in grub and other preboot menus? Status in QEMU: Invalid Bug description: Currently virtio-input devices cannot be used before the kernel loads. This is not really a full bug but it would be much more useful if you can use the keyboard+mouse this way. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1618265/+subscriptions
[Bug 1845580] Re: issue with QEMU on Raspberry Pi failing to access CDROM
All right, so this was the known issue. Let's close this bug :-) ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1845580 Title: issue with QEMU on Raspberry Pi failing to access CDROM Status in QEMU: Fix Released Bug description: I am trying to access the CDROM (iso) from QEMU using FreeDOS and I get an error when doing a directory for: i can boot from the iso but if i exit to access the files from the CDROM ISO i get the attached error. I believe there is an issue with the QEMU for the Raspberry Pi. I am using a Raspberry Pi3 with the latest full Raspbian Buster load To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1845580/+subscriptions
Re: [PATCH v2 29/33] spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate
On Mon, Sep 30, 2019 at 09:29:14PM +0200, Cédric Le Goater wrote: > On 30/09/2019 10:25, David Gibson wrote: > > On Mon, Sep 30, 2019 at 08:11:56AM +0200, Cédric Le Goater wrote: > >> On 27/09/2019 07:50, David Gibson wrote: > >>> It turns out that all the logic in the SpaprIrq::reset hooks (and some in > >>> the SpaprIrq::post_load hooks) isn't really related to resetting the irq > >>> backend (that's handled by the backends' own reset routines). Rather its > >>> about getting the backend ready to be the active interrupt controller or > >>> stopping being the active interrupt controller - reset (and post_load) is > >>> just the only time that changes at present. > >> > >> This is a 'critical' part which impacts all the migration cases: > >> > >> ic-mode=xics,xive,dual + kernel_irqchip=on/off + TCG > > > > Yes... and? > > and it's fragile. How so? Explicitly having logic for when an intc becomes active or inactive, and having a single callsite which does that and updates the active controller seems less fragile to me. At least compared to having the update to the active controller (implicit in changing the CAS vector) and the logic to get the controllers ready for being active/inactive in totally different places and relying on the fact they both only happen at reset for them to travel together. > > >>> To make this flow clearer, move the logic into the explicit backend > >>> activate and deactivate hooks. > >> > >> I don't see where the hooks are called ? > > > > spapr_irq_reset() > > -> spapr_irq_update_active_intc() > > -> set_active_intc() > > -> activate/deactivate hooks > > > > Similarly via spapr_irq_post_load(). > > > > I'm hoping to add one at CAS time to avoid the CAS reboot, too. > > OK. I think the first 22 patches are ready, just some minor comments > if I am correct. Could you merge them ? I might repost first, because one of the changes Greg suggested to error handling caused a larger than expected number of ripple on fixes. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH v2 21/33] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController
On Mon, Sep 30, 2019 at 12:13:14PM +0200, Cédric Le Goater wrote:
> On 30/09/2019 08:14, David Gibson wrote:
> > On Mon, Sep 30, 2019 at 07:28:45AM +0200, Cédric Le Goater wrote:
> >> On 30/09/2019 03:49, David Gibson wrote:
> >>> On Fri, Sep 27, 2019 at 12:16:49PM +0200, Greg Kurz wrote:
> On Fri, 27 Sep 2019 15:50:16 +1000
> David Gibson wrote:
>
> > This method essentially represents code which belongs to the interrupt
> > controller, but needs to be called on all possible intcs, rather than
> > just the currently active one. The "dual" version therefore calls
> > into the xics and xive versions confusingly.
> >
> > Handle this more directly, by making it instead a method on the intc
> > backend, and always calling it on every backend that exists.
> >
> > While we're there, streamline the error reporting a bit.
> >
> > Signed-off-by: David Gibson
> >>> [snip]
> > @@ -525,6 +469,30 @@ static void spapr_irq_check(SpaprMachineState
> > *spapr, Error **errp)
> > /*
> > * sPAPR IRQ frontend routines for devices
> > */
> > +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr,
> > + PowerPCCPU *cpu, Error **errp)
> > +{
> > +if (spapr->xive) {
> > +SpaprInterruptController *intc = SPAPR_INTC(spapr->xive);
> > +SpaprInterruptControllerClass *sicc =
> > SPAPR_INTC_GET_CLASS(intc);
> > +
> > +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
> > +return -1;
> > +}
> > +}
> > +
> > +if (spapr->ics) {
> > +SpaprInterruptController *intc = SPAPR_INTC(spapr->ics);
> > +SpaprInterruptControllerClass *sicc =
> > SPAPR_INTC_GET_CLASS(intc);
> > +
> > +if (sicc->cpu_intc_create(intc, cpu, errp) < 0) {
> > +return -1;
> > +}
> > +}
> > +
>
> Instead of these hooks, what about open-coding
> spapr_xive_cpu_intc_create()
> and xics_spapr_cpu_intc_create() directly here, like you already did for
> the
> ICS and the XIVE objects in spapr_irq_init() ?
> >>>
> >>> I'd prefer not to. The idea is I want to treat this as basically:
> >>>
> >>> foreach_possible_intc(intc)
> >>> intc::cpu_intc_create(...)
> >>>
> >>> If I find time I might indeed replace the explicit ics and xive
> >>> pointers with just an array of SpaprInterruptController *.
> >>
> >> Or you could use object_child_foreach() and check for the type. If we had
> >> a helper object_child_foreach_type(), we could use it elsewhere.
> >
> > I thought about that, but I don't think it quite works. The
> > complication is that the xics device is made explicitly a child of the
> > machine, but the xive device has mmio, so it's a SusBusDevice sitting
> > on the root bus instead.
>
> PnvXscom works fine with Devices and SysBusDevices.
Uh... what's an example of it working with a SysBusDevice? All the
implementors of PNV_XSCOM_INTERFACE I could find were instantiated
with object_initialize_child() making them explicitly children of the
chip. The SPAPR_XIVE is instantiated with qdev_create(NULL,
TYPE_SPAPR_XIVE), making it a child of the root bus, not the machine,
I believe.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Bug 1845580] Re: issue with QEMU on Raspberry Pi failing to access CDROM
Thank you for the info. I installed the gtk2 and sdl2 development libraries recompiled and 4.1.0 now runs successfully and that also fixed the original issue I was seeing. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1845580 Title: issue with QEMU on Raspberry Pi failing to access CDROM Status in QEMU: New Bug description: I am trying to access the CDROM (iso) from QEMU using FreeDOS and I get an error when doing a directory for: i can boot from the iso but if i exit to access the files from the CDROM ISO i get the attached error. I believe there is an issue with the QEMU for the Raspberry Pi. I am using a Raspberry Pi3 with the latest full Raspbian Buster load To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1845580/+subscriptions
Re: [PATCH 06/20] xics: Create sPAPR specific ICS subtype
On Mon, Sep 30, 2019 at 07:00:43PM +0200, Greg Kurz wrote: > On Mon, 30 Sep 2019 18:45:30 +1000 > David Gibson wrote: > > > On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > > > On Thu, 26 Sep 2019 10:56:46 +1000 > > > David Gibson wrote: > > > > > > > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > > > > On 25/09/2019 10:40, Greg Kurz wrote: > > > > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > > > > David Gibson wrote: > > > > > > > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now > > > > > >> all this > > > > > >> does is move the setup of the PAPR specific hcalls and RTAS calls > > > > > >> to > > > > > >> the realize() function for this, rather than requiring the PAPR > > > > > >> code to > > > > > >> explicitly call xics_spapr_init(). In future it will have some > > > > > >> more > > > > > >> function. > > > > > >> > > > > > >> Signed-off-by: David Gibson > > > > > >> --- > > > > > > > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState > > > > > > typedef > > > > > > > > > > why ? we have ICS_SPAPR() for the checks already. > > > > > > > > Eh.. using typedefs when we haven't actually extended a base type > > > > isn't common QOM practice. Yes, it's not as typesafe as it could be, > > > > but I'm not really inclined to go to the extra effort here. > > > > > > I'll soon need to extend the base type with a nr_servers field, > > > > Uh.. nr_servers doesn't seem like it belongs in the base ICS type. > > Of course ! I re-used the wording "extended a base type" of your sentence, > that I understand as "a subtype extends a base type with some more data". > I'm talking about the sPAPR ICS subtype here, not the base ICS type. Ah, ok. > > That really would conflict with the pnv usage where the ICS is > > supposed to just represent the ICS, not the xics as a whole. If you > > need nr_servers information here, it seems like pulling it via a > > method in XICSFabric would make more sense. > > > > > and while here with an fd field as well in order to get rid of > > > the ugly global in xics.c. I'll go to the extra effort :) > > > > That could go in the derived type. We already kind of conflate ICS > > and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR > > only anyway. > > > > Exactly, so that's why I was thinking about adding nr_servers there, > but it could go to XICSFabric as well I guess. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
About the idle driver for calling MWAIT in QEMU/KVM Guest without VM exits.
Hi Michael and Paolo, I read your patch[1] about better MWAIT emulation in the QEMU/KVM Guest. As shown in [1], you mentioned that you were testing and would post the idle driver calling MWAIT in the QEMU/KVM Guest to avoid VM exits. However, I could not find that idle driver. I appreciate if you can post the idle driver and how to use it in the QEMU/KVM Guest. Or, you may want to give me some suggestions about how to use MWAIT in the QEMU/KVM Guest. Thanks much for your help. [1] https://patchwork.kernel.org/patch/9674991/ Best, Hary
Re: [RFC] cpu_map: Remove pconfig from Icelake-Server CPU model
On Mon, 2019-09-30 at 17:16 +0200, Paolo Bonzini wrote: > On 30/09/19 16:31, Hu, Robert wrote: > > > This might be a problem if there are plans to eventually make KVM support > > > pconfig, though. Paolo, Robert, are there plans to support pconfig in KVM > > > in the > > > future? > > [Robert Hoo] > > Thanks Eduardo for efforts in resolving this issue, introduced from my > > Icelake CPU > > model patch. > > I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai > > and answer > > you soon. > > It's really, really unlikely. It's possible that some future processor > overloads PCONFIG in such a way that it will become virtualizable, but > not IceLake. I agree. Not Icelake. Thanks, -Kai > > Would it make sense for libvirt to treat absent CPU flags as "default > off" during migration, so that it can leave out the flag in the command > line if it's off? If it's on, libvirt would pass pconfig=on as usual. > This is a variant of [2], but more generally applicable: > > > [2] However starting a domain with Icelake-Server so that it can be > > migrated or saved/restored on QEMU in 3.1.1 and 4.0.0 would be > > impossible. This can be solved by a different hack, which would drop > > pconfig=off from QEMU command line. > > Paolo
Re: [PATCH] x86: Add CPUID KVM support for new instruction WBNOINVD
On Mon, 2019-09-30 at 12:23 -0700, Jim Mattson wrote: > On Mon, Sep 30, 2019 at 10:54 AM Eduardo Habkost wrote: > > CCing qemu-devel. > > > > On Tue, Sep 24, 2019 at 01:30:04PM -0700, Jim Mattson wrote: > > > On Wed, Dec 19, 2018 at 1:02 PM Paolo Bonzini wrote: > > > > On 19/12/18 18:39, Jim Mattson wrote: > > > > > Is this an instruction that kvm has to be able to emulate before it > > > > > can enumerate its existence? > > > > > > > > It doesn't have any operands, so no. > > > > > > > > Paolo > > > > > > > > > On Wed, Dec 19, 2018 at 5:51 AM Robert Hoo > > > > > wrote: > > > > > > Signed-off-by: Robert Hoo > > > > > > --- > > > > > > arch/x86/include/asm/cpufeatures.h | 1 + > > > > > > arch/x86/kvm/cpuid.c | 2 +- > > > > > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/arch/x86/include/asm/cpufeatures.h > > > > > > b/arch/x86/include/asm/cpufeatures.h > > > > > > index 28c4a50..932b19f 100644 > > > > > > --- a/arch/x86/include/asm/cpufeatures.h > > > > > > +++ b/arch/x86/include/asm/cpufeatures.h > > > > > > @@ -280,6 +280,7 @@ > > > > > > /* AMD-defined CPU features, CPUID level 0x8008 (EBX), word 13 > > > > > > */ > > > > > > #define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO > > > > > > instruction */ > > > > > > #define X86_FEATURE_IRPERF (13*32+ 1) /* Instructions > > > > > > Retired Count */ > > > > > > +#define X86_FEATURE_WBNOINVD (13*32+ 9) /* Writeback and > > > > > > Don't invalid cache */ > > > > > > #define X86_FEATURE_XSAVEERPTR (13*32+ 2) /* Always > > > > > > save/restore FP error pointers */ > > > > > > #define X86_FEATURE_AMD_IBPB (13*32+12) /* "" Indirect > > > > > > Branch Prediction Barrier */ > > > > > > #define X86_FEATURE_AMD_IBRS (13*32+14) /* "" Indirect > > > > > > Branch Restricted Speculation */ > > > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > > > > > index cc6dd65..763e115 100644 > > > > > > --- a/arch/x86/kvm/cpuid.c > > > > > > +++ b/arch/x86/kvm/cpuid.c > > > > > > @@ -380,7 +380,7 @@ static inline int __do_cpuid_ent(struct > > > > > > kvm_cpuid_entry2 *entry, u32 function, > > > > > > > > > > > > /* cpuid 0x8008.ebx */ > > > > > > const u32 kvm_cpuid_8000_0008_ebx_x86_features = > > > > > > - F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | > > > > > > F(VIRT_SSBD) | > > > > > > + F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | > > > > > > F(AMD_SSBD) | F(VIRT_SSBD) | > > > > > > F(AMD_SSB_NO) | F(AMD_STIBP); > > > > > > > > > > > > /* cpuid 0xC001.edx */ > > > > > > -- > > > > > > 1.8.3.1 > > > > > > > > > > > > What is the point of enumerating support for WBNOINVD if kvm is going > > > to implement it as WBINVD? > > > > I expect GET_SUPPORTED_CPUID to return WBNOINVD, because it > > indicates to userspace what is supported by KVM. Are there any > > expectations that GET_SUPPORTED_CPUID will also dictate what is > > enabled by default in some cases? > > > > In either case, your question applies to QEMU: why do we want > > WBNOINVD to be enabled by "-cpu host" by default and be part of > > QEMU's Icelake-* CPU model definitions? > > I had only looked at the SVM implementation of WBNOINVD, which is > exactly the same as the SVM implementation of WBINVD. So, the question > is, "why enumerate WBNOINVD if its implementation is exactly the same > as WBINVD?" > > WBNOINVD appears to be only partially documented in Intel document > 319433-037, "Intel® Architecture Instruction Set Extensions and Future > Features Programming Reference." In particular, there is no > documentation regarding the instruction's behavior in VMX non-root > mode. Does WBNOINVD cause a VM-exit when the VM-execution control, > "WBINVD exiting," is set? If so, does it have the same VM-exit reason > as WBINVD (54), or a different one? If it does have the same VM-exit > reason (a la SVM), how does one distinguish a WBINVD VM-exit from a > WBNOINVD VM-exit? If one can't distinguish (a la SVM), then it would > seem that the VMX implementation also implements WBNOINVD as WBINVD. > If that's the case, the question for VMX is the same as for SVM. Unfortunately WBNOINVD interaction with VMX has not been made to public yet. I am reaching out internally to see when it can be done. I agree it may not be necessary to expose WBNOINVD if its implementation is exactly the same as WBINVD, but it also doesn't have any harm, right? Thanks, -Kai
Re: Qemu Dirty Bitmap backup to encrypted target
On 9/30/19 3:26 PM, Craig Mull wrote:
> How can have QEMU backup write the output to an encrypted target?
>
> Blocks in the dirty bitmap are unencrypted, and as such when I write
> them with QEMU backup they are written to the target unencrypted.
>
> I've experimented with providing a json string as the target but with no
> luck.
>
>
> transaction='{ "execute": "transaction",
>
> "arguments": {
>
> "actions": [
>
> {"type": "block-dirty-bitmap-add",
>
> "data": {"node": "drive-virtio-disk0", "granularity": 2097152,
> "name": "mybitmap"} },
>
> {"type": "drive-backup",
>
> "data": {"device": "drive-virtio-disk0", "target":
> "json:{\"encrypt.format\": \"luks\", \"encrypt.key-secret\":
> \"virtio-disk0-luks-secret0\", \"driver\": \"qcow2\", \"file\":
> {\"driver\": \"file\", \"filename\": \"/tmp/target-encrypt-test.qcow2\"}}",
>
> "sync": "full", "format": "qcow2"} }
>
> ]
>
> }
>
> }'
>
>
>
> virsh -c qemu:///system qemu-monitor-command --pretty 28 $transaction
>
>
>
> {
>
> "id": "libvirt-45",
>
> "error": {
>
> "class": "GenericError",
>
> "desc": "Unknown protocol 'json'"
>
> }
>
> }
>
>
I'll be honest, I'm not very good at the json specifications and don't
really know when they're appropriate to use. At the basic level,
drive-backup expects a filename. Sometimes the filename can get fancy,
but... I stay away from that.
Try using qmp-blockdev-create to create the target node instead, and
then using blockdev-backup to backup to that target.
--js
Re: Minor regression with ramfb+gvt-g+edk2 with qemu 4.1
On Thu, 2019-09-26 at 13:51 +0300, Maxim Levitsky wrote: > Hi! > > I noticed that with qemu 4.1, when I try to open the > UEFI bios screen, the screen output is garbaged. > > Basicaly what happens is this: > > ramfb driver inside the uefi bios is hardcoded to initialize > the resolution to 800x600 > > Then when you enter the BIOS screen, it switches the resolution > to the 'preferred resolution' which by default is 640x480 > > However since commit a9e0cb67b7f4c485755659f9b764c38b5f970de4, > ramfb resolution is locked once it is set once, thus, > the switch to 640x480 is ignored. > > Best regards, > Maxim Levitsky Ping! Best regards, Maxim Levitsky
bitmap migration bug with -drive while block mirror runs
Hi folks, I identified a problem with the migration code that Red Hat QE found and thought you'd like to see it: https://bugzilla.redhat.com/show_bug.cgi?id=1652424#c20 Very, very briefly: drive-mirror inserts a filter node that changes what bdrv_get_device_or_node_name() returns, which causes a migration problem. Ignorant question #1: Can we multi-parent the filter node and source-node? It looks like at the moment both consider their only parent to be the block-job and don't have a link back to their parents otherwise. Otherwise: I have a lot of cloudy ideas on how to solve this, but ultimately what we want is to be able to find the "addressable" name for the node the bitmap is attached to, which would be the name of the first ancestor node that isn't a filter. (OR, the name of the block-backend above that node.) A simple way to do this might be a "child_unfiltered" BdrvChild role that simply bypasses the filter that was inserted and serves no real purpose other than to allow the child to have a parent link and find who it's """real""" parent is. Because of flushing, reopen, sync, drain &c &c &c I'm not sure how feasible this quick idea might be, though. - Corollary fix #1: call error_setg if the bitmap node name that's about to go over the wire is an autogenerated node: this is never correct! (Why not? because the target is incapable of matching the node-name because they are randomly generated AND you cannot specify node-names with # prefixes as they are especially reserved! (This raises a related problem: if you explicitly add bitmaps to nodes with autogenerated names, you will be unable to migrate them.)) --js
Re: [PATCH 0/1] RFC: implement reopen for nbd driver
Patchew URL: https://patchew.org/QEMU/20190930213820.29777-1-mlevi...@redhat.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190930213820.29777-1-mlevi...@redhat.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: Is kexec supported in QEMU for ARM64 (qemu-system-aarch64) with arm-trusted-firmware, optee, and u-boot.
On 9/27/19 2:15 AM, Philippe Mathieu-Daudé wrote: Cc'ing Ard too https://salsa.debian.org/debian/atf-allwinner/commit/b6b671c4ac4bd5595306863225bb3bece1e6135c Current limitations: * Only cold boot is supported * No build instructions for QEMU_EFI.fd and rootfs-arm64.cpio.gz * No instructions for how to load a BL32 (Secure Payload) So looks like only cold boot is supported (no kexec support) Is this correct? Just wanted to check again - Does ATF and QEMU (for ARM64) support cold boot only and does not have support for kexec? thanks, -lakshmi
Re: [PATCH v5 00/10] Audio: Mixeng-free 5.1/7.1 audio support
Patchew URL: https://patchew.org/QEMU/cover.1569874641.git.dirty.ice...@gmail.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/cover.1569874641.git.dirty.ice...@gmail.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v7 00/22] tcg/ppc: Add vector opcodes
Patchew URL: https://patchew.org/QEMU/20190930202125.21064-1-richard.hender...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20190930202125.21064-1-richard.hender...@linaro.org Subject: [PATCH v7 00/22] tcg/ppc: Add vector opcodes === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20190930213820.29777-1-mlevi...@redhat.com -> patchew/20190930213820.29777-1-mlevi...@redhat.com Switched to a new branch 'test' 478e76c tcg/ppc: Update vector support for v3.00 dup/dupi b89afa6 tcg/ppc: Update vector support for v3.00 load/store bce09e8 tcg/ppc: Update vector support for v3.00 Altivec d5f2f79 tcg/ppc: Update vector support for v2.07 FP e0fc728 tcg/ppc: Update vector support for v2.07 VSX b1be698 tcg/ppc: Update vector support for v2.07 Altivec 5fc86b4 tcg/ppc: Update vector support for VSX 89107c6 tcg/ppc: Enable Altivec detection b3d5636 tcg/ppc: Support vector dup2 47e0e2b tcg/ppc: Support vector multiply 1993d36 tcg/ppc: Support vector shift by immediate cbeffa5 tcg/ppc: Add support for vector saturated add/subtract 1241ea6 tcg/ppc: Add support for vector add/subtract f9972ad tcg/ppc: Add support for vector maximum/minimum 13da469 tcg/ppc: Add support for load/store/logic/comparison 5ff43ce tcg/ppc: Enable tcg backend vector compilation 96e0364 tcg/ppc: Replace HAVE_ISEL macro with a variable 9c0980c tcg/ppc: Replace HAVE_ISA_2_06 e3266d3 tcg/ppc: Create TCGPowerISA and have_isa 998bca4 tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC() 6272fbe tcg/ppc: Introduce macro VX4() 0035792 tcg/ppc: Introduce Altivec registers === OUTPUT BEGIN === 1/22 Checking commit 00357923b0b1 (tcg/ppc: Introduce Altivec registers) 2/22 Checking commit 6272fbed21ab (tcg/ppc: Introduce macro VX4()) ERROR: spaces required around that '|' (ctx:VxV) #21: FILE: tcg/ppc/tcg-target.inc.c:322: +#define VX4(opc) (OPCD(4)|(opc)) ^ total: 1 errors, 0 warnings, 7 lines checked Patch 2/22 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/22 Checking commit 998bca481a97 (tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC()) 4/22 Checking commit e3266d393526 (tcg/ppc: Create TCGPowerISA and have_isa) 5/22 Checking commit 9c0980c45e5d (tcg/ppc: Replace HAVE_ISA_2_06) 6/22 Checking commit 96e03641f26f (tcg/ppc: Replace HAVE_ISEL macro with a variable) 7/22 Checking commit 5ff43ce33249 (tcg/ppc: Enable tcg backend vector compilation) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #163: new file mode 100644 total: 0 errors, 1 warnings, 129 lines checked Patch 7/22 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 8/22 Checking commit 13da469a4ee1 (tcg/ppc: Add support for load/store/logic/comparison) 9/22 Checking commit f9972ad57906 (tcg/ppc: Add support for vector maximum/minimum) 10/22 Checking commit 1241ea67c7b2 (tcg/ppc: Add support for vector add/subtract) 11/22 Checking commit cbeffa5f1b81 (tcg/ppc: Add support for vector saturated add/subtract) 12/22 Checking commit 1993d365c640 (tcg/ppc: Support vector shift by immediate) 13/22 Checking commit 47e0e2b46bbd (tcg/ppc: Support vector multiply) ERROR: code indent should never use tabs #133: FILE: tcg/ppc/tcg-target.inc.c:3217: +^Ibreak;$ total: 1 errors, 0 warnings, 192 lines checked Patch 13/22 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 14/22 Checking commit b3d5636c8887 (tcg/ppc: Support vector dup2) 15/22 Checking commit 89107c63bb12 (tcg/ppc: Enable Altivec detection) 16/22 Checking commit 5fc86b499068 (tcg/ppc: Update vector support for VSX) 17/22 Checking commit b1be69897458 (tcg/ppc: Update vector support for v2.07 Altivec) 18/22 Checking commit e0fc72804bd0 (tcg/ppc: Update vector support for v2.07 VSX) 19/22 Checking commit d5f2f795d0d8 (tcg/ppc: Update vector support for v2.07 FP) 20/22 Checking commit bce09e83ceff (tcg/ppc: Update vector support for v3.00 Altivec) 21/22 Checking commit b89afa6b8a8d (tcg/ppc: Update vector support for v3.00 load/store) 22/22 Checking commit 478e76cf962b (tcg/ppc: Update vector support for v3.00 dup/dupi) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190930202125.21064-1-richard.hender...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please
[PATCH 0/1] RFC: implement reopen for nbd driver
Hi, It looks like nbd driver doesn't have support for reopen, which doesn't allow to commit qcow2 snapshots which have nbd export as a base file. This is because the base is opened read-only, and only when commit job starts it reopens the base read-write. Now after talking with Eric Blake, I understood that nbd doesn't have the ability to tell the server to open read/only and then change this on the fly, thus even when opening an export as read-only the server will still allow writes. This means that an empty .bdrv_reopen_prepare (well except checking that export is not read-only) is supposed to be enough. Sending this as RFC, since I am not sure that this is the correct solution. Best regards, Maxim Levitsky Maxim Levitsky (1): nbd: add empty .bdrv_reopen_prepare block/nbd.c | 15 +++ 1 file changed, 15 insertions(+) -- 2.17.2
[PATCH 1/1] nbd: add empty .bdrv_reopen_prepare
Fixes commit job / qemu-img commit, when
commiting qcow2 file which is based on nbd export.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1718727
Signed-off-by: Maxim Levitsky
---
block/nbd.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index 813c40d8f0..fd78e5f330 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1158,6 +1158,18 @@ static int coroutine_fn nbd_client_co_block_status(
BDRV_BLOCK_OFFSET_VALID;
}
+static int nbd_client_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+BDRVNBDState *s = (BDRVNBDState *)state->bs->opaque;
+
+if ((state->flags & BDRV_O_RDWR) && (s->info.flags & NBD_FLAG_READ_ONLY)) {
+error_setg(errp, "Can't reopen read-only NBD mount as read/write");
+return -EACCES;
+}
+return 0;
+}
+
static void nbd_client_close(BlockDriverState *bs)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
@@ -1798,6 +1810,7 @@ static BlockDriver bdrv_nbd = {
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename= nbd_parse_filename,
.bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev= nbd_client_co_pwritev,
.bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
@@ -1820,6 +1833,7 @@ static BlockDriver bdrv_nbd_tcp = {
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename= nbd_parse_filename,
.bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev= nbd_client_co_pwritev,
.bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
@@ -1842,6 +1856,7 @@ static BlockDriver bdrv_nbd_unix = {
.instance_size = sizeof(BDRVNBDState),
.bdrv_parse_filename= nbd_parse_filename,
.bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
.bdrv_co_preadv = nbd_client_co_preadv,
.bdrv_co_pwritev= nbd_client_co_pwritev,
.bdrv_co_pwrite_zeroes = nbd_client_co_pwrite_zeroes,
--
2.17.2
Re: Arch info lost in "info cpus"
On Mon, Sep 30, 2019 at 12:22:22PM +0200, Sergio Lopez wrote: > > Alex Bennée writes: > > > Sergio Lopez writes: > > > >> Hi, > >> > >> Commit 137b5cb6ab565cb3781d5337591e155932b4230e (hmp: change > >> hmp_info_cpus to use query-cpus-fast) updated the "info cpus" commit to > >> make it more lightweight, but also removed the ability to get the > >> architecture specific status of each vCPU. > >> > >> This information was really useful to diagnose certain Guest issues, > >> without the need of using GDB, which is more intrusive and requires > >> enabling it in advance. > > > > You can always enable the gdbserver from the HMP when you need it. > > > >> Is there an alternative way of getting something equivalent to what > >> "info cpus" provided previously (in 2.10)? > > > > info registers > > > > should give you a full dump of the CPU state including the PC. > > > > Both methods are less convenient that what we had before. Perhaps it'd > be reasonable adding a flag to "info cpus" to give users the options of > having the same behavior as before? Is "info registers -a" less convenient because it prints too much information, or because it doesn't print the active CPU indicator and thread_id? -- Eduardo
Re: [PATCH] user-exec: Do not filter the signal on si_code
On 9/30/19 12:29 PM, Richard Henderson wrote:
> This is a workaround for a ppc64le host kernel bug.
>
> For the test case linux-test, we have an instruction trace
>
> IN: sig_alarm
> ...
>
> IN:
> 0x400080ed28: 38ac li r0, 0xac
> 0x400080ed2c: 4402 sc
>
> IN: __libc_nanosleep
> 0x1003bb4c: 7c0802a6 mflr r0
> 0x1003bb50: f8010010 std r0, 0x10(r1)
>
> Our signal return trampoline has, rightly, changed the guest
> stack page read-only. Which, rightly, faults on the store of
> a return address into a stack frame.
>
> Checking the host /proc/pid/maps, we see the expected state:
>
> 400080-400081 r--p 00:00 0
>
> However, the host kernel has supplied si_code == SEGV_MAPERR,
> which is obviously incorrect.
>
> By dropping this check, we may have an extra walk of the page
> tables, but this should be inexpensive.
>
> Signed-off-by: Richard Henderson
> ---
>
> FWIW, filed as
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1757189
>
> out of habit and then
>
> https://bugs.centos.org/view.php?id=16499
>
> when I remembered that the system is running Centos not RHEL.
>
> ---
> accel/tcg/user-exec.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 71c4bf6477..31ef091a70 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -143,9 +143,12 @@ static inline int handle_cpu_signal(uintptr_t pc,
> siginfo_t *info,
> * for some other kind of fault that should really be passed to the
> * guest, we'd end up in an infinite loop of retrying the faulting
> * access.
> + *
> + * XXX: At least one host kernel, ppc64le w/Centos 7 4.14.0-115.6.1,
> + * incorrectly reports SEGV_MAPERR for a STDX write to a read-only page.
> + * Therefore, do not test info->si_code.
> */
> -if (is_write && info->si_signo == SIGSEGV && info->si_code ==
> SEGV_ACCERR &&
> -h2g_valid(address)) {
> +if (is_write && info->si_signo == SIGSEGV && h2g_valid(address)) {
Ho hum. This change is in conflict with Peter's long comment; I should have
read the context more thoroughly. There is an even longer comment with the
patch description: 9c4bbee9e3b83544257e82566342c29e15a88637
The SEGV_ACCERR check here is to prevent a loop by which page_unprotect races
with itself and, from Peter's analysis,
> * ...but when B gets the mmap lock it finds that the page is already
>PAGE_WRITE, and so it exits page_unprotect() via the "not due to
>protected translation" code path, and wrongly delivers the signal
>to the guest rather than just retrying the access
This bug was fixed in the referenced patch. But then continues:
> Since this would cause an infinite loop if we ever called
> page_unprotect() for some other kind of fault than "write failed due
> to bad access permissions", tighten the condition in
> handle_cpu_signal() to check the signal number and si_code, and add a
> comment so that if somebody does ever find themselves debugging an
> infinite loop of faults they have some clue about why.
>
> (The trick for identifying the correct setting for
> current_tb_invalidated for thread B (needed to handle the precise-SMC
> case) is due to Richard Henderson. Paolo Bonzini suggested just
> relying on si_code rather than trying anything more complicated.)
It is disappointing about the kernel bug. But since this affects Centos 7,
which is what *all* of the gcc compile farm ppc64 machines use, I think we need
to work around it somehow.
Should we simply add SEGV_MAPERR to the set of allowed si_code, to directly
work around the bug? If we got that code from a kernel without the bug, then
page_find should fail to find an entry, and we should then indicate that the
signal should be passed to the guest.
Thoughts?
r~
[PATCH v5 10/10] paaudio: fix channel order for usb-audio 5.1 and 7.1 streams
Signed-off-by: Kővágó, Zoltán
---
audio/paaudio.c | 50 -
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/audio/paaudio.c b/audio/paaudio.c
index d195b1caa8..6ff0d17537 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -338,17 +338,59 @@ static pa_stream *qpa_simple_new (
pa_stream_direction_t dir,
const char *dev,
const pa_sample_spec *ss,
-const pa_channel_map *map,
const pa_buffer_attr *attr,
int *rerror)
{
int r;
-pa_stream *stream;
+pa_stream *stream = NULL;
pa_stream_flags_t flags;
+pa_channel_map map;
pa_threaded_mainloop_lock(c->mainloop);
-stream = pa_stream_new(c->context, name, ss, map);
+pa_channel_map_init(&map);
+map.channels = ss->channels;
+
+/*
+ * TODO: This currently expects the only frontend supporting more than 2
+ * channels is the usb-audio. We will need some means to set channel
+ * order when a new frontend gains multi-channel support.
+ */
+switch (ss->channels) {
+case 1:
+map.map[0] = PA_CHANNEL_POSITION_MONO;
+break;
+
+case 2:
+map.map[0] = PA_CHANNEL_POSITION_LEFT;
+map.map[1] = PA_CHANNEL_POSITION_RIGHT;
+break;
+
+case 6:
+map.map[0] = PA_CHANNEL_POSITION_FRONT_LEFT;
+map.map[1] = PA_CHANNEL_POSITION_FRONT_RIGHT;
+map.map[2] = PA_CHANNEL_POSITION_CENTER;
+map.map[3] = PA_CHANNEL_POSITION_LFE;
+map.map[4] = PA_CHANNEL_POSITION_REAR_LEFT;
+map.map[5] = PA_CHANNEL_POSITION_REAR_RIGHT;
+break;
+
+case 8:
+map.map[0] = PA_CHANNEL_POSITION_FRONT_LEFT;
+map.map[1] = PA_CHANNEL_POSITION_FRONT_RIGHT;
+map.map[2] = PA_CHANNEL_POSITION_CENTER;
+map.map[3] = PA_CHANNEL_POSITION_LFE;
+map.map[4] = PA_CHANNEL_POSITION_REAR_LEFT;
+map.map[5] = PA_CHANNEL_POSITION_REAR_RIGHT;
+map.map[6] = PA_CHANNEL_POSITION_SIDE_LEFT;
+map.map[7] = PA_CHANNEL_POSITION_SIDE_RIGHT;
+
+default:
+dolog("Internal error: unsupported channel count %d\n", ss->channels);
+goto fail;
+}
+
+stream = pa_stream_new(c->context, name, ss, &map);
if (!stream) {
goto fail;
}
@@ -421,7 +463,6 @@ static int qpa_init_out(HWVoiceOut *hw, struct audsettings
*as,
PA_STREAM_PLAYBACK,
ppdo->has_name ? ppdo->name : NULL,
&ss,
-NULL, /* channel map */
&ba,/* buffering attributes */
&error
);
@@ -470,7 +511,6 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings
*as, void *drv_opaque)
PA_STREAM_RECORD,
ppdo->has_name ? ppdo->name : NULL,
&ss,
-NULL, /* channel map */
&ba,/* buffering attributes */
&error
);
--
2.23.0
[PATCH v5 07/10] usb-audio: do not count on avail bytes actually available
This assumption is no longer true when mixeng is turned off.
Signed-off-by: Kővágó, Zoltán
---
hw/usb/dev-audio.c | 28 +---
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index ae42e5a2f1..74c99b1f12 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -319,30 +319,31 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p)
{
uint32_t free = buf->size - (buf->prod - buf->cons);
-if (!free) {
+if (free < USBAUDIO_PACKET_SIZE) {
return 0;
}
if (p->iov.size != USBAUDIO_PACKET_SIZE) {
return 0;
}
-assert(free >= USBAUDIO_PACKET_SIZE);
+
usb_packet_copy(p, buf->data + (buf->prod % buf->size),
USBAUDIO_PACKET_SIZE);
buf->prod += USBAUDIO_PACKET_SIZE;
return USBAUDIO_PACKET_SIZE;
}
-static uint8_t *streambuf_get(struct streambuf *buf)
+static uint8_t *streambuf_get(struct streambuf *buf, size_t *len)
{
uint32_t used = buf->prod - buf->cons;
uint8_t *data;
if (!used) {
+*len = 0;
return NULL;
}
-assert(used >= USBAUDIO_PACKET_SIZE);
data = buf->data + (buf->cons % buf->size);
-buf->cons += USBAUDIO_PACKET_SIZE;
+*len = MIN(buf->prod - buf->cons,
+ buf->size - (buf->cons % buf->size));
return data;
}
@@ -374,16 +375,21 @@ static void output_callback(void *opaque, int avail)
USBAudioState *s = opaque;
uint8_t *data;
-for (;;) {
-if (avail < USBAUDIO_PACKET_SIZE) {
+while (avail) {
+size_t written, len;
+
+data = streambuf_get(&s->out.buf, &len);
+if (!data) {
return;
}
-data = streambuf_get(&s->out.buf);
-if (!data) {
+
+written = AUD_write(s->out.voice, data, len);
+avail -= written;
+s->out.buf.cons += written;
+
+if (written < len) {
return;
}
-AUD_write(s->out.voice, data, USBAUDIO_PACKET_SIZE);
-avail -= USBAUDIO_PACKET_SIZE;
}
}
--
2.23.0
[PATCH v5 04/10] audio: support more than two channels in volume setting
Signed-off-by: Kővágó, Zoltán
---
audio/audio.c | 30 ++
audio/audio.h | 10 ++
audio/audio_int.h | 4 ++--
audio/paaudio.c| 20
audio/spiceaudio.c | 14 --
5 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index d616a4af98..f1c145dfcd 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1891,31 +1891,45 @@ void AUD_del_capture (CaptureVoiceOut *cap, void
*cb_opaque)
}
void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+Volume vol = { .mute = mute, .channels = 2, .vol = { lvol, rvol } };
+audio_set_volume_out(sw, &vol);
+}
+
+void audio_set_volume_out(SWVoiceOut *sw, Volume *vol)
{
if (sw) {
HWVoiceOut *hw = sw->hw;
-sw->vol.mute = mute;
-sw->vol.l = nominal_volume.l * lvol / 255;
-sw->vol.r = nominal_volume.r * rvol / 255;
+sw->vol.mute = vol->mute;
+sw->vol.l = nominal_volume.l * vol->vol[0] / 255;
+sw->vol.r = nominal_volume.l * vol->vol[vol->channels > 1 ? 1 : 0] /
+255;
if (hw->pcm_ops->volume_out) {
-hw->pcm_ops->volume_out(hw, &sw->vol);
+hw->pcm_ops->volume_out(hw, vol);
}
}
}
void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+Volume vol = { .mute = mute, .channels = 2, .vol = { lvol, rvol } };
+audio_set_volume_in(sw, &vol);
+}
+
+void audio_set_volume_in(SWVoiceIn *sw, Volume *vol)
{
if (sw) {
HWVoiceIn *hw = sw->hw;
-sw->vol.mute = mute;
-sw->vol.l = nominal_volume.l * lvol / 255;
-sw->vol.r = nominal_volume.r * rvol / 255;
+sw->vol.mute = vol->mute;
+sw->vol.l = nominal_volume.l * vol->vol[0] / 255;
+sw->vol.r = nominal_volume.r * vol->vol[vol->channels > 1 ? 1 : 0] /
+255;
if (hw->pcm_ops->volume_in) {
-hw->pcm_ops->volume_in(hw, &sw->vol);
+hw->pcm_ops->volume_in(hw, vol);
}
}
}
diff --git a/audio/audio.h b/audio/audio.h
index c74abb8c47..0db3c7dd5e 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -124,6 +124,16 @@ uint64_t AUD_get_elapsed_usec_out (SWVoiceOut *sw,
QEMUAudioTimeStamp *ts);
void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol);
void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol);
+#define AUDIO_MAX_CHANNELS 16
+typedef struct Volume {
+bool mute;
+int channels;
+uint8_t vol[AUDIO_MAX_CHANNELS];
+} Volume;
+
+void audio_set_volume_out(SWVoiceOut *sw, Volume *vol);
+void audio_set_volume_in(SWVoiceIn *sw, Volume *vol);
+
SWVoiceIn *AUD_open_in (
QEMUSoundCard *card,
SWVoiceIn *sw,
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 22a703c13e..9176db249b 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -166,7 +166,7 @@ struct audio_pcm_ops {
*/
size_t (*put_buffer_out)(HWVoiceOut *hw, void *buf, size_t size);
void (*enable_out)(HWVoiceOut *hw, bool enable);
-void (*volume_out)(HWVoiceOut *hw, struct mixeng_volume *vol);
+void (*volume_out)(HWVoiceOut *hw, Volume *vol);
int(*init_in) (HWVoiceIn *hw, audsettings *as, void *drv_opaque);
void (*fini_in) (HWVoiceIn *hw);
@@ -174,7 +174,7 @@ struct audio_pcm_ops {
void *(*get_buffer_in)(HWVoiceIn *hw, size_t *size);
void (*put_buffer_in)(HWVoiceIn *hw, void *buf, size_t size);
void (*enable_in)(HWVoiceIn *hw, bool enable);
-void (*volume_in)(HWVoiceIn *hw, struct mixeng_volume *vol);
+void (*volume_in)(HWVoiceIn *hw, Volume *vol);
};
void *audio_generic_get_buffer_in(HWVoiceIn *hw, size_t *size);
diff --git a/audio/paaudio.c b/audio/paaudio.c
index 6ccdf31415..d195b1caa8 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -531,20 +531,22 @@ static void qpa_fini_in (HWVoiceIn *hw)
}
}
-static void qpa_volume_out(HWVoiceOut *hw, struct mixeng_volume *vol)
+static void qpa_volume_out(HWVoiceOut *hw, Volume *vol)
{
PAVoiceOut *pa = (PAVoiceOut *) hw;
pa_operation *op;
pa_cvolume v;
PAConnection *c = pa->g->conn;
+int i;
#ifdef PA_CHECK_VERSION/* macro is present in 0.9.16+ */
pa_cvolume_init (&v); /* function is present in 0.9.13+ */
#endif
-v.channels = 2;
-v.values[0] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->l) / UINT32_MAX;
-v.values[1] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->r) / UINT32_MAX;
+v.channels = vol->channels;
+for (i = 0; i < vol->channels; ++i) {
+v.values[i] = ((PA_VOLUME_NORM - PA_VOLUME_MUTED) * vol->vol[i]) / 255;
+}
pa_threaded_mainloop_lock(c->mainloop);
@@ -571,20 +573,22 @@ static void qpa_volume_out(HWVoiceOut *hw, struct
mixeng_volume *vol)
pa_threaded_mainloop_unlock(c->mainloop);
}
-static void qpa_volume_in(HWVoiceIn *hw, struct mixeng_volume *vol)
+static void qpa_vol
Re: Lockup with --accel tcg,thread=single
On 30/09/19 21:20, Alex Bennée wrote:
> Does seem to imply the vCPU CPUState is where the queue is. That's not
> to say there shouldn't be a single work queue for thread=single.
Indeed it doesn't. I confused this with commit a8efa60633 ("cpus: run
work items for all vCPUs if single-threaded", 2018-11-27).
Are you going to make a patch to have a single work queue, or should I?
Paolo
[PATCH v5 02/10] audio: make mixeng optional
Implementation of the previously added mixing-engine option.
Signed-off-by: Kővágó, Zoltán
---
Notes:
Changes from v4:
* audio_pcm_hw_add_* always returns a new HW (or fails) when not using
mixeng
audio/audio.c | 70 ++
audio/audio_template.h | 24 ++-
2 files changed, 80 insertions(+), 14 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 7128ee98dc..d616a4af98 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -838,32 +838,46 @@ static void audio_timer (void *opaque)
*/
size_t AUD_write(SWVoiceOut *sw, void *buf, size_t size)
{
+HWVoiceOut *hw;
+
if (!sw) {
/* XXX: Consider options */
return size;
}
+hw = sw->hw;
-if (!sw->hw->enabled) {
+if (!hw->enabled) {
dolog ("Writing to disabled voice %s\n", SW_NAME (sw));
return 0;
}
-return audio_pcm_sw_write(sw, buf, size);
+if (audio_get_pdo_out(hw->s->dev)->mixing_engine) {
+return audio_pcm_sw_write(sw, buf, size);
+} else {
+return hw->pcm_ops->write(hw, buf, size);
+}
}
size_t AUD_read(SWVoiceIn *sw, void *buf, size_t size)
{
+HWVoiceIn *hw;
+
if (!sw) {
/* XXX: Consider options */
return size;
}
+hw = sw->hw;
-if (!sw->hw->enabled) {
+if (!hw->enabled) {
dolog ("Reading from disabled voice %s\n", SW_NAME (sw));
return 0;
}
-return audio_pcm_sw_read(sw, buf, size);
+if (audio_get_pdo_in(hw->s->dev)->mixing_engine) {
+return audio_pcm_sw_read(sw, buf, size);
+} else {
+return hw->pcm_ops->read(hw, buf, size);
+}
}
int AUD_get_buffer_size_out (SWVoiceOut *sw)
@@ -1090,6 +1104,26 @@ static void audio_run_out (AudioState *s)
HWVoiceOut *hw = NULL;
SWVoiceOut *sw;
+if (!audio_get_pdo_out(s->dev)->mixing_engine) {
+while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
+/* there is exactly 1 sw for each hw with no mixeng */
+sw = hw->sw_head.lh_first;
+
+if (hw->pending_disable) {
+hw->enabled = 0;
+hw->pending_disable = 0;
+if (hw->pcm_ops->enable_out) {
+hw->pcm_ops->enable_out(hw, false);
+}
+}
+
+if (sw->active) {
+sw->callback.fn(sw->callback.opaque, INT_MAX);
+}
+}
+return;
+}
+
while ((hw = audio_pcm_hw_find_any_enabled_out(s, hw))) {
size_t played, live, prev_rpos, free;
int nb_live, cleanup_required;
@@ -1227,6 +1261,17 @@ static void audio_run_in (AudioState *s)
{
HWVoiceIn *hw = NULL;
+if (!audio_get_pdo_in(s->dev)->mixing_engine) {
+while ((hw = audio_pcm_hw_find_any_enabled_in(s, hw))) {
+/* there is exactly 1 sw for each hw with no mixeng */
+SWVoiceIn *sw = hw->sw_head.lh_first;
+if (sw->active) {
+sw->callback.fn(sw->callback.opaque, INT_MAX);
+}
+}
+return;
+}
+
while ((hw = audio_pcm_hw_find_any_enabled_in(s, hw))) {
SWVoiceIn *sw;
size_t captured = 0, min;
@@ -1751,6 +1796,11 @@ CaptureVoiceOut *AUD_add_capture(
s = audio_init(NULL, NULL);
}
+if (!audio_get_pdo_out(s->dev)->mixing_engine) {
+dolog("Can't capture with mixeng disabled\n");
+return NULL;
+}
+
if (audio_validate_settings (as)) {
dolog ("Invalid settings were passed when trying to add capture\n");
audio_print_settings (as);
@@ -1905,9 +1955,13 @@ void audio_create_pdos(Audiodev *dev)
static void audio_validate_per_direction_opts(
AudiodevPerDirectionOptions *pdo, Error **errp)
{
+if (!pdo->has_mixing_engine) {
+pdo->has_mixing_engine = true;
+pdo->mixing_engine = true;
+}
if (!pdo->has_fixed_settings) {
pdo->has_fixed_settings = true;
-pdo->fixed_settings = true;
+pdo->fixed_settings = pdo->mixing_engine;
}
if (!pdo->fixed_settings &&
(pdo->has_frequency || pdo->has_channels || pdo->has_format)) {
@@ -1915,6 +1969,10 @@ static void audio_validate_per_direction_opts(
"You can't use frequency, channels or format with
fixed-settings=off");
return;
}
+if (!pdo->mixing_engine && pdo->fixed_settings) {
+error_setg(errp, "You can't use fixed-settings without mixeng");
+return;
+}
if (!pdo->has_frequency) {
pdo->has_frequency = true;
@@ -1926,7 +1984,7 @@ static void audio_validate_per_direction_opts(
}
if (!pdo->has_voices) {
pdo->has_voices = true;
-pdo->voices = 1;
+pdo->voices = pdo->mixing_engine ? 1 : INT_MAX;
}
if (!pdo->has_format) {
pdo->has_format = true;
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 235d1acbbe..3709a4431c 100
[PATCH v5 03/10] paaudio: get/put_buffer functions
This lets us avoid some buffer copying when using mixeng.
Signed-off-by: Kővágó, Zoltán
---
audio/paaudio.c | 83 +
1 file changed, 83 insertions(+)
diff --git a/audio/paaudio.c b/audio/paaudio.c
index ed31f863f7..6ccdf31415 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -98,6 +98,59 @@ static inline int PA_STREAM_IS_GOOD(pa_stream_state_t x)
} \
} while (0)
+static void *qpa_get_buffer_in(HWVoiceIn *hw, size_t *size)
+{
+PAVoiceIn *p = (PAVoiceIn *) hw;
+PAConnection *c = p->g->conn;
+int r;
+
+pa_threaded_mainloop_lock(c->mainloop);
+
+CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
+"pa_threaded_mainloop_lock failed\n");
+
+if (!p->read_length) {
+r = pa_stream_peek(p->stream, &p->read_data, &p->read_length);
+CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail,
+ "pa_stream_peek failed\n");
+}
+
+*size = MIN(p->read_length, *size);
+
+pa_threaded_mainloop_unlock(c->mainloop);
+return (void *) p->read_data;
+
+unlock_and_fail:
+pa_threaded_mainloop_unlock(c->mainloop);
+*size = 0;
+return NULL;
+}
+
+static void qpa_put_buffer_in(HWVoiceIn *hw, void *buf, size_t size)
+{
+PAVoiceIn *p = (PAVoiceIn *) hw;
+PAConnection *c = p->g->conn;
+int r;
+
+pa_threaded_mainloop_lock(c->mainloop);
+
+CHECK_DEAD_GOTO(c, p->stream, unlock,
+"pa_threaded_mainloop_lock failed\n");
+
+assert(buf == p->read_data && size <= p->read_length);
+
+p->read_data += size;
+p->read_length -= size;
+
+if (size && !p->read_length) {
+r = pa_stream_drop(p->stream);
+CHECK_SUCCESS_GOTO(c, r == 0, unlock, "pa_stream_drop failed\n");
+}
+
+unlock:
+pa_threaded_mainloop_unlock(c->mainloop);
+}
+
static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length)
{
PAVoiceIn *p = (PAVoiceIn *) hw;
@@ -136,6 +189,32 @@ unlock_and_fail:
return 0;
}
+static void *qpa_get_buffer_out(HWVoiceOut *hw, size_t *size)
+{
+PAVoiceOut *p = (PAVoiceOut *) hw;
+PAConnection *c = p->g->conn;
+void *ret;
+int r;
+
+pa_threaded_mainloop_lock(c->mainloop);
+
+CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail,
+"pa_threaded_mainloop_lock failed\n");
+
+*size = -1;
+r = pa_stream_begin_write(p->stream, &ret, size);
+CHECK_SUCCESS_GOTO(c, r >= 0, unlock_and_fail,
+ "pa_stream_begin_write failed\n");
+
+pa_threaded_mainloop_unlock(c->mainloop);
+return ret;
+
+unlock_and_fail:
+pa_threaded_mainloop_unlock(c->mainloop);
+*size = 0;
+return NULL;
+}
+
static size_t qpa_write(HWVoiceOut *hw, void *data, size_t length)
{
PAVoiceOut *p = (PAVoiceOut *) hw;
@@ -698,11 +777,15 @@ static struct audio_pcm_ops qpa_pcm_ops = {
.init_out = qpa_init_out,
.fini_out = qpa_fini_out,
.write= qpa_write,
+.get_buffer_out = qpa_get_buffer_out,
+.put_buffer_out = qpa_write, /* pa handles it */
.volume_out = qpa_volume_out,
.init_in = qpa_init_in,
.fini_in = qpa_fini_in,
.read = qpa_read,
+.get_buffer_in = qpa_get_buffer_in,
+.put_buffer_in = qpa_put_buffer_in,
.volume_in = qpa_volume_in
};
--
2.23.0
[PATCH v5 08/10] usb-audio: support more than two channels of audio
This commit adds support for 5.1 and 7.1 audio playback. This commit
adds a new property to usb-audio:
* multi=on|off
Whether to enable the 5.1 and 7.1 audio support. When off (default)
it continues to emulate the old stereo-only device. When on, it
emulates a slightly different audio device that supports 5.1 and 7.1
audio.
Signed-off-by: Kővágó, Zoltán
---
Notes:
According to the spec the channel order is front left, front right,
center, lfe, surround left, surround right for 5.1 sound. But alsa uses
front left, front right, surround left, surround right, center, lfe.
This means if you simply use an alsa device as -audiodev, the surround
and center/lfe channels will be swapped. There is not much to do sort
of writing a mini mixeng or something like that, but you can easily add
a new device to /etc/asound.conf that swaps the channels:
pcm.swap {
type route
slave.pcm "default" # or whatever
slave.channels 6
ttable.0.0 1
ttable.1.1 1
ttable.2.4 1
ttable.3.5 1
ttable.4.2 1
ttable.5.3 1
}
and use -audiodev alsa,id=foo,out.mixing-engine=off,alsa-out.dev=swap,...
(due to how usb and usb-audio works, you'll probably need
alsa-out.try-poll=off and some playing with threshold)
Pulseaudio allows us to specify channel order, and we default to a
compatible order so it should work correctly without extra setup.
hw/usb/dev-audio.c | 419 +++--
1 file changed, 366 insertions(+), 53 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 74c99b1f12..e42bdfbdc1 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -37,11 +37,15 @@
#include "desc.h"
#include "audio/audio.h"
+static void usb_audio_reinit(USBDevice *dev, unsigned channels);
+
#define USBAUDIO_VENDOR_NUM 0x46f4 /* CRC16() of "QEMU" */
#define USBAUDIO_PRODUCT_NUM0x0002
#define DEV_CONFIG_VALUE1 /* The one and only */
+#define USBAUDIO_MAX_CHANNELS(s) (s->multi ? 8 : 2)
+
/* Descriptor subtypes for AC interfaces */
#define DST_AC_HEADER 1
#define DST_AC_INPUT_TERMINAL 2
@@ -80,6 +84,27 @@ static const USBDescStrings usb_audio_stringtable = {
[STRING_REAL_STREAM]= "Audio Output - 48 kHz Stereo",
};
+/*
+ * A USB audio device supports an arbitrary number of alternate
+ * interface settings for each interface. Each corresponds to a block
+ * diagram of parameterized blocks. This can thus refer to things like
+ * number of channels, data rates, or in fact completely different
+ * block diagrams. Alternative setting 0 is always the null block diagram,
+ * which is used by a disabled device.
+ */
+enum usb_audio_altset {
+ALTSET_OFF= 0x00, /* No endpoint */
+ALTSET_STEREO = 0x01, /* Single endpoint */
+ALTSET_51 = 0x02,
+ALTSET_71 = 0x03,
+};
+
+static unsigned altset_channels[] = {
+[ALTSET_STEREO] = 2,
+[ALTSET_51] = 6,
+[ALTSET_71] = 8,
+};
+
#define U16(x) ((x) & 0xff), (((x) >> 8) & 0xff)
#define U24(x) U16(x), (((x) >> 16) & 0xff)
#define U32(x) U24(x), (((x) >> 24) & 0xff)
@@ -87,7 +112,8 @@ static const USBDescStrings usb_audio_stringtable = {
/*
* A Basic Audio Device uses these specific values
*/
-#define USBAUDIO_PACKET_SIZE 192
+#define USBAUDIO_PACKET_SIZE_BASE 96
+#define USBAUDIO_PACKET_SIZE(channels) (USBAUDIO_PACKET_SIZE_BASE * channels)
#define USBAUDIO_SAMPLE_RATE 48000
#define USBAUDIO_PACKET_INTERVAL 1
@@ -121,7 +147,7 @@ static const USBDescIface desc_iface[] = {
0x01, /* u8 bTerminalID */
U16(0x0101),/* u16 wTerminalType */
0x00, /* u8 bAssocTerminal */
-0x02, /* u16 bNrChannels */
+0x02, /* u8 bNrChannels */
U16(0x0003),/* u16 wChannelConfig */
0x00, /* u8 iChannelNames */
STRING_INPUT_TERMINAL, /* u8 iTerminal */
@@ -156,14 +182,14 @@ static const USBDescIface desc_iface[] = {
},
},{
.bInterfaceNumber = 1,
-.bAlternateSetting = 0,
+.bAlternateSetting = ALTSET_OFF,
.bNumEndpoints = 0,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSubClass= USB_SUBCLASS_AUDIO_STREAMING,
.iInterface= STRING_NULL_STREAM,
},{
.bInterfaceNumber = 1,
-.bAlternateSetting = 1,
+.bAlternateSetting = ALTSET_STEREO,
.bNumEndpoints = 1,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSub
[PATCH v5 05/10] audio: replace shift in audio_pcm_info with bytes_per_frame
The bit shifting trick worked because the number of bytes per frame was
always a power-of-two (since QEMU only supports mono, stereo and 8, 16
and 32 bit samples). But if we want to add support for surround sound,
this no longer holds true.
Signed-off-by: Kővágó, Zoltán
---
audio/alsaaudio.c | 11 +++---
audio/audio.c | 74 -
audio/audio_int.h | 3 +-
audio/coreaudio.c | 4 +--
audio/dsound_template.h | 10 +++---
audio/dsoundaudio.c | 4 +--
audio/noaudio.c | 2 +-
audio/ossaudio.c| 14
audio/spiceaudio.c | 3 +-
audio/wavaudio.c| 6 ++--
10 files changed, 66 insertions(+), 65 deletions(-)
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index cfe42284a6..eddf013a53 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -602,7 +602,7 @@ static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t
len)
{
ALSAVoiceOut *alsa = (ALSAVoiceOut *) hw;
size_t pos = 0;
-size_t len_frames = len >> hw->info.shift;
+size_t len_frames = len / hw->info.bytes_per_frame;
while (len_frames) {
char *src = advance(buf, pos);
@@ -648,7 +648,7 @@ static size_t alsa_write(HWVoiceOut *hw, void *buf, size_t
len)
}
}
-pos += written << hw->info.shift;
+pos += written * hw->info.bytes_per_frame;
if (written < len_frames) {
break;
}
@@ -802,7 +802,8 @@ static size_t alsa_read(HWVoiceIn *hw, void *buf, size_t
len)
void *dst = advance(buf, pos);
snd_pcm_sframes_t nread;
-nread = snd_pcm_readi(alsa->handle, dst, len >> hw->info.shift);
+nread = snd_pcm_readi(
+alsa->handle, dst, len / hw->info.bytes_per_frame);
if (nread <= 0) {
switch (nread) {
@@ -828,8 +829,8 @@ static size_t alsa_read(HWVoiceIn *hw, void *buf, size_t
len)
}
}
-pos += nread << hw->info.shift;
-len -= nread << hw->info.shift;
+pos += nread * hw->info.bytes_per_frame;
+len -= nread * hw->info.bytes_per_frame;
}
return pos;
diff --git a/audio/audio.c b/audio/audio.c
index f1c145dfcd..c00f4deddd 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -299,12 +299,13 @@ static int audio_pcm_info_eq (struct audio_pcm_info
*info, struct audsettings *a
void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as)
{
-int bits = 8, sign = 0, shift = 0;
+int bits = 8, sign = 0, mul;
switch (as->fmt) {
case AUDIO_FORMAT_S8:
sign = 1;
case AUDIO_FORMAT_U8:
+mul = 1;
break;
case AUDIO_FORMAT_S16:
@@ -312,7 +313,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
/* fall through */
case AUDIO_FORMAT_U16:
bits = 16;
-shift = 1;
+mul = 2;
break;
case AUDIO_FORMAT_S32:
@@ -320,7 +321,7 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
/* fall through */
case AUDIO_FORMAT_U32:
bits = 32;
-shift = 2;
+mul = 4;
break;
default:
@@ -331,9 +332,8 @@ void audio_pcm_init_info (struct audio_pcm_info *info,
struct audsettings *as)
info->bits = bits;
info->sign = sign;
info->nchannels = as->nchannels;
-info->shift = (as->nchannels == 2) + shift;
-info->align = (1 << info->shift) - 1;
-info->bytes_per_second = info->freq << info->shift;
+info->bytes_per_frame = as->nchannels * mul;
+info->bytes_per_second = info->freq * info->bytes_per_frame;
info->swap_endianness = (as->endianness != AUDIO_HOST_ENDIANNESS);
}
@@ -344,26 +344,25 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info
*info, void *buf, int len)
}
if (info->sign) {
-memset (buf, 0x00, len << info->shift);
+memset(buf, 0x00, len * info->bytes_per_frame);
}
else {
switch (info->bits) {
case 8:
-memset (buf, 0x80, len << info->shift);
+memset(buf, 0x80, len * info->bytes_per_frame);
break;
case 16:
{
int i;
uint16_t *p = buf;
-int shift = info->nchannels - 1;
short s = INT16_MAX;
if (info->swap_endianness) {
s = bswap16 (s);
}
-for (i = 0; i < len << shift; i++) {
+for (i = 0; i < len * info->nchannels; i++) {
p[i] = s;
}
}
@@ -373,14 +372,13 @@ void audio_pcm_info_clear_buf (struct audio_pcm_info
*info, void *buf, int len)
{
int i;
uint32_t *p = buf;
-int shift = info->nchannels - 1;
int32_t s = INT32_MAX;
if (info->swap_endianness) {
s
[PATCH v5 01/10] audio: add mixing-engine option (documentation)
This will allow us to disable mixeng when we use a decent backend.
Disabling mixeng have a few advantages:
* we no longer convert the audio output from one format to another, when
the underlying audio system would just convert it to a third format.
We no longer convert, only the underlying system, when needed.
* the underlying system probably has better resampling and sample format
converting methods anyway...
* we may support formats that the mixeng currently does not support (S24
or float samples, more than two channels)
* when using an audio server (like pulseaudio) different sound card
outputs will show up as separate streams, even if we use only one
backend
Disadvantages:
* audio capturing no longer works (wavcapture, and vnc audio extension)
* some backends only support a single playback stream or very picky
about the audio format. In this case we can't disable mixeng.
However mixeng is not removed, only made optional, so this shouldn't be
a big concern.
Signed-off-by: Kővágó, Zoltán
---
Notes:
Changes from v1:
* renamed mixeng to mixing-engine
qapi/audio.json | 5 +
qemu-options.hx | 6 ++
2 files changed, 11 insertions(+)
diff --git a/qapi/audio.json b/qapi/audio.json
index 9fefdf5186..0996705954 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -11,6 +11,10 @@
# General audio backend options that are used for both playback and
# recording.
#
+# @mixing-engine: use QEMU's mixing engine to mix all streams inside QEMU. When
+# set to off, fixed-settings must be also off. Not every
backend
+# is compatible with the off setting (default on, since 4.2)
+#
# @fixed-settings: use fixed settings for host input/output. When off,
# frequency, channels and format must not be
# specified (default true)
@@ -31,6 +35,7 @@
##
{ 'struct': 'AudiodevPerDirectionOptions',
'data': {
+'*mixing-engine': 'bool',
'*fixed-settings': 'bool',
'*frequency': 'uint32',
'*channels': 'uint32',
diff --git a/qemu-options.hx b/qemu-options.hx
index 2a04ca6ac5..3e50956f48 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -433,6 +433,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
"specifies the audio backend to use\n"
"id= identifier of the backend\n"
"timer-period= timer period in microseconds\n"
+"in|out.mixing-engine= use mixing engine to mix streams
inside QEMU\n"
"in|out.fixed-settings= use fixed settings for host
audio\n"
"in|out.frequency= frequency to use with fixed settings\n"
"in|out.channels= number of channels to use with fixed
settings\n"
@@ -503,6 +504,11 @@ Identifies the audio backend.
Sets the timer @var{period} used by the audio subsystem in microseconds.
Default is 1 (10 ms).
+@item in|out.mixing-engine=on|off
+Use QEMU's mixing engine to mix all streams inside QEMU. When off,
+@var{fixed-settings} must be off too. Not every backend is fully
+compatible with the off setting. Default is on.
+
@item in|out.fixed-settings=on|off
Use fixed settings for host audio. When off, it will change based on
how the guest opens the sound card. In this case you must not specify
--
2.23.0
[PATCH v5 09/10] usbaudio: change playback counters to 64 bit
With stereo playback, they need about 375 minutes of continuous audio
playback to overflow, which is usually not a problem (as stopping and
later resuming playback resets the counters). But with 7.1 audio, they
only need about 95 minutes to overflow.
After the overflow, the buf->prod % USBAUDIO_PACKET_SIZE(channels)
assertion no longer holds true, which will result in overflowing the
buffer. With 64 bit variables, it would take about 762000 years to
overflow.
Signed-off-by: Kővágó, Zoltán
---
hw/usb/dev-audio.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index e42bdfbdc1..ea604bbb8e 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -578,9 +578,9 @@ static const USBDesc desc_audio_multi = {
struct streambuf {
uint8_t *data;
-uint32_t size;
-uint32_t prod;
-uint32_t cons;
+size_t size;
+uint64_t prod;
+uint64_t cons;
};
static void streambuf_init(struct streambuf *buf, uint32_t size,
@@ -601,7 +601,7 @@ static void streambuf_fini(struct streambuf *buf)
static int streambuf_put(struct streambuf *buf, USBPacket *p, uint32_t
channels)
{
-uint32_t free = buf->size - (buf->prod - buf->cons);
+int64_t free = buf->size - (buf->prod - buf->cons);
if (free < USBAUDIO_PACKET_SIZE(channels)) {
return 0;
@@ -610,6 +610,8 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p, uint32_t channels)
return 0;
}
+/* can happen if prod overflows */
+assert(buf->prod % USBAUDIO_PACKET_SIZE(channels) == 0);
usb_packet_copy(p, buf->data + (buf->prod % buf->size),
USBAUDIO_PACKET_SIZE(channels));
buf->prod += USBAUDIO_PACKET_SIZE(channels);
@@ -618,10 +620,10 @@ static int streambuf_put(struct streambuf *buf, USBPacket
*p, uint32_t channels)
static uint8_t *streambuf_get(struct streambuf *buf, size_t *len)
{
-uint32_t used = buf->prod - buf->cons;
+int64_t used = buf->prod - buf->cons;
uint8_t *data;
-if (!used) {
+if (used <= 0) {
*len = 0;
return NULL;
}
--
2.23.0
Re: Thoughts on VM fence infrastructure
> On Sep 30, 2019, at 8:45 PM, Rafael David Tinoco > wrote: > > There are times when the main loop can get blocked even though the CPU threads can be running and can in some configurations perform IO even without the main loop (I think!). >>> Ah, that's a very good point. Indeed, you can perform IO in those >>> cases specially when using vhost devices. >>> By setting a timer in the kernel that sends a signal to qemu, the kernel will send that signal however broken qemu is. >>> Got you now. That's probably better. Do you reckon a signal is >>> preferable over SIGEV_THREAD? >> Not sure; probably the safest is getting the kernel to SIGKILL it - but >> that's a complete nightmare to debug - your process just goes *pop* >> with no apparent reason why. >> I've not used SIGEV_THREAD - it looks promising though. > > Sorry to "enter" the discussion, but, in "real" HW, its not by accident > that watchdog devices timeout generates a NMI to CPUs, causing the > kernel to handle the interrupt - and panic (or to take other action set > by specific watchdog drivers that re-implements the default ones). Not sure what you mean by "sorry"... thanks for joining. :) > Can't you simple "inject" a NMI in all guest vCPUs BEFORE you take any > action in QEMU itself? Just like the virtual watchdog device would do, > from inside the guest (/dev/watchdog), but capable of being updated by > outside, in this case of yours (if I understood correctly). It's unclear to me how this relates to this use case, perhaps that's not clear. The idea is that on various cloud deployments, a host could be temporarily unavailable. Imagine that a network cable snapped. A management layer could then restart the unreachable VMs elsewhere (as part of High Availability offerings), but it needs to ensure that disconnected host is not just going to come back from the dead with older incarnations of the VMs running. (Imagine that someone replaced the broken network cable.) That would result in lots of issues from colliding IP addresses to different writers on shared storage leading to data corruption. The ask is for a mechanism to fence the host, essentially causing all (or selected) VMs on that host to die. There are several mechanisms for that, mostly requiring some sort of HW support (eg. STONITH). Those are often focused on cases where the host requires manual intervention to recover or at least a reset. I'm looking to implement a mechanism for self-fencing, which doesn't require external hardware and cover most failure scenarios (from partially/totally broken hosts to simply a temporary network failure). In several cases rebooting the host is unnecessary; just ensuring the VMs are down is enough. That's almost always true on temporary network unavailability (eg. split network). > Possibly you would have to have a dedicated loop for this "watchdog > device" (AIO threads ?) not to compete with existing coroutines/BH Tasks > and their jittering on your "realtime watchdog needs". Only when this feature is needed (which isn't the case for most people), there would be an extra thread (according to the latest proposal) which is mostly idle. It would wake up every few seconds and stat() a file, which is a very lightweight operation. That would not measurably impact/jitter other work. > Regarding remaining existing I/OS for the guest's devices in question > (vhost/vhost-user etc), would be just like a real host where the "bus" > received commands, but sender died right after... I hope the above clarifies the idea. Cheers, F.
[PATCH v7 22/22] tcg/ppc: Update vector support for v3.00 dup/dupi
These new instructions are conditional on MSR.VEC for TX=1,
so we can consider these Altivec instructions.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 5b7d1bd2dc..d308d69aba 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -596,11 +596,14 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define XXPERMDI (OPCD(60) | (10 << 3) | 7) /* v2.06, force ax=bx=tx=1 */
#define XXSEL (OPCD(60) | (3 << 4) | 0xf) /* v2.06, force ax=bx=cx=tx=1 */
+#define XXSPLTIB (OPCD(60) | (360 << 1) | 1) /* v3.00, force tx=1 */
#define MFVSRD (XO31(51) | 1) /* v2.07, force sx=1 */
#define MFVSRWZ(XO31(115) | 1) /* v2.07, force sx=1 */
#define MTVSRD (XO31(179) | 1) /* v2.07, force tx=1 */
#define MTVSRWZ(XO31(243) | 1) /* v2.07, force tx=1 */
+#define MTVSRDD(XO31(435) | 1) /* v3.00, force tx=1 */
+#define MTVSRWS(XO31(403) | 1) /* v3.00, force tx=1 */
#define RT(r) ((r)<<21)
#define RS(r) ((r)<<21)
@@ -931,6 +934,10 @@ static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
TCGReg ret,
return;
}
}
+if (have_isa_3_00 && val == (tcg_target_long)dup_const(MO_8, val)) {
+tcg_out32(s, XXSPLTIB | VRT(ret) | ((val & 0xff) << 11));
+return;
+}
/*
* Otherwise we must load the value from the constant pool.
@@ -3021,7 +3028,22 @@ static bool tcg_out_dup_vec(TCGContext *s, TCGType type,
unsigned vece,
TCGReg dst, TCGReg src)
{
tcg_debug_assert(dst >= TCG_REG_V0);
-tcg_debug_assert(src >= TCG_REG_V0);
+
+/* Splat from integer reg allowed via constraints for v3.00. */
+if (src < TCG_REG_V0) {
+tcg_debug_assert(have_isa_3_00);
+switch (vece) {
+case MO_64:
+tcg_out32(s, MTVSRDD | VRT(dst) | RA(src) | RB(src));
+return true;
+case MO_32:
+tcg_out32(s, MTVSRWS | VRT(dst) | RA(src));
+return true;
+default:
+/* Fail, so that we fall back on either dupm or mov+dup. */
+return false;
+}
+}
/*
* Recall we use (or emulate) VSX integer loads, so the integer is
@@ -3482,6 +3504,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
static const TCGTargetOpDef sub2
= { .args_ct_str = { "r", "r", "rI", "rZM", "r", "r" } };
static const TCGTargetOpDef v_r = { .args_ct_str = { "v", "r" } };
+static const TCGTargetOpDef v_vr = { .args_ct_str = { "v", "vr" } };
static const TCGTargetOpDef v_v = { .args_ct_str = { "v", "v" } };
static const TCGTargetOpDef v_v_v = { .args_ct_str = { "v", "v", "v" } };
static const TCGTargetOpDef v_v_v_v
@@ -3651,8 +3674,9 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
return &v_v_v;
case INDEX_op_not_vec:
case INDEX_op_neg_vec:
-case INDEX_op_dup_vec:
return &v_v;
+case INDEX_op_dup_vec:
+return have_isa_3_00 ? &v_vr : &v_v;
case INDEX_op_ld_vec:
case INDEX_op_st_vec:
case INDEX_op_dupm_vec:
--
2.17.1
[PATCH v7 16/22] tcg/ppc: Update vector support for VSX
The VSX instruction set instructions include double-word loads and
stores, double-word load and splat, double-word permute, and bit
select. All of which require multiple operations in the Altivec
instruction set.
Because the VSX registers map %vsr32 to %vr0, and we have no current
intention or need to use vector registers outside %vr0-%vr19, force
on the {ax,bx,cx,tx} bits within the added VSX insns so that we don't
have to otherwise modify the VR[TABC] macros.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 5 ++--
tcg/ppc/tcg-target.inc.c | 52
2 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index f50b7f4bac..c974ca274a 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -66,6 +66,7 @@ typedef enum {
extern TCGPowerISA have_isa;
extern bool have_altivec;
+extern bool have_vsx;
#define have_isa_2_06 (have_isa >= tcg_isa_2_06)
#define have_isa_3_00 (have_isa >= tcg_isa_3_00)
@@ -149,7 +150,7 @@ extern bool have_altivec;
* instruction and substituting two 32-bit stores makes the generated
* code quite large.
*/
-#define TCG_TARGET_HAS_v64 0
+#define TCG_TARGET_HAS_v64 have_vsx
#define TCG_TARGET_HAS_v128 have_altivec
#define TCG_TARGET_HAS_v256 0
@@ -165,7 +166,7 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_mul_vec 1
#define TCG_TARGET_HAS_sat_vec 1
#define TCG_TARGET_HAS_minmax_vec 1
-#define TCG_TARGET_HAS_bitsel_vec 0
+#define TCG_TARGET_HAS_bitsel_vec have_vsx
#define TCG_TARGET_HAS_cmpsel_vec 0
void flush_icache_range(uintptr_t start, uintptr_t stop);
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index d739f4b605..2388958405 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -67,6 +67,7 @@ static tcg_insn_unit *tb_ret_addr;
TCGPowerISA have_isa;
static bool have_isel;
bool have_altivec;
+bool have_vsx;
#ifndef CONFIG_SOFTMMU
#define TCG_GUEST_BASE_REG 30
@@ -467,9 +468,12 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define LVEBX XO31(7)
#define LVEHX XO31(39)
#define LVEWX XO31(71)
+#define LXSDX (XO31(588) | 1) /* v2.06, force tx=1 */
+#define LXVDSX (XO31(332) | 1) /* v2.06, force tx=1 */
#define STVX XO31(231)
#define STVEWX XO31(199)
+#define STXSDX (XO31(716) | 1) /* v2.06, force sx=1 */
#define VADDSBSVX4(768)
#define VADDUBSVX4(512)
@@ -558,6 +562,9 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VSLDOI VX4(44)
+#define XXPERMDI (OPCD(60) | (10 << 3) | 7) /* v2.06, force ax=bx=tx=1 */
+#define XXSEL (OPCD(60) | (3 << 4) | 0xf) /* v2.06, force ax=bx=cx=tx=1 */
+
#define RT(r) ((r)<<21)
#define RS(r) ((r)<<21)
#define RA(r) ((r)<<16)
@@ -884,11 +891,21 @@ static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
TCGReg ret,
add = 0;
}
-load_insn = LVX | VRT(ret) | RB(TCG_REG_TMP1);
-if (TCG_TARGET_REG_BITS == 64) {
-new_pool_l2(s, rel, s->code_ptr, add, val, val);
+if (have_vsx) {
+load_insn = type == TCG_TYPE_V64 ? LXSDX : LXVDSX;
+load_insn |= VRT(ret) | RB(TCG_REG_TMP1);
+if (TCG_TARGET_REG_BITS == 64) {
+new_pool_label(s, val, rel, s->code_ptr, add);
+} else {
+new_pool_l2(s, rel, s->code_ptr, add, val, val);
+}
} else {
-new_pool_l4(s, rel, s->code_ptr, add, val, val, val, val);
+load_insn = LVX | VRT(ret) | RB(TCG_REG_TMP1);
+if (TCG_TARGET_REG_BITS == 64) {
+new_pool_l2(s, rel, s->code_ptr, add, val, val);
+} else {
+new_pool_l4(s, rel, s->code_ptr, add, val, val, val, val);
+}
}
if (USE_REG_TB) {
@@ -1136,6 +1153,10 @@ static void tcg_out_ld(TCGContext *s, TCGType type,
TCGReg ret,
/* fallthru */
case TCG_TYPE_V64:
tcg_debug_assert(ret >= TCG_REG_V0);
+if (have_vsx) {
+tcg_out_mem_long(s, 0, LXSDX, ret, base, offset);
+break;
+}
tcg_debug_assert((offset & 7) == 0);
tcg_out_mem_long(s, 0, LVX, ret, base, offset & -16);
if (offset & 8) {
@@ -1180,6 +1201,10 @@ static void tcg_out_st(TCGContext *s, TCGType type,
TCGReg arg,
/* fallthru */
case TCG_TYPE_V64:
tcg_debug_assert(arg >= TCG_REG_V0);
+if (have_vsx) {
+tcg_out_mem_long(s, 0, STXSDX, arg, base, offset);
+break;
+}
tcg_debug_assert((offset & 7) == 0);
if (offset & 8) {
tcg_out_vsldoi(s, TCG_VEC_TMP1, arg, arg, 8);
@@ -2899,6 +2924,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_shri_vec:
case INDEX_op_sari_vec:
return vece <= MO_32 ? -1 : 0
[PATCH v7 17/22] tcg/ppc: Update vector support for v2.07 Altivec
These new instructions are conditional only on MSR.VEC and
are thus part of the Altivec instruction set, and not VSX.
This includes lots of double-word arithmetic and a few extra
logical operations.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.h | 4 +-
tcg/ppc/tcg-target.inc.c | 85 ++--
2 files changed, 67 insertions(+), 22 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index c974ca274a..13197eddce 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -61,6 +61,7 @@ typedef enum {
typedef enum {
tcg_isa_base,
tcg_isa_2_06,
+tcg_isa_2_07,
tcg_isa_3_00,
} TCGPowerISA;
@@ -69,6 +70,7 @@ extern bool have_altivec;
extern bool have_vsx;
#define have_isa_2_06 (have_isa >= tcg_isa_2_06)
+#define have_isa_2_07 (have_isa >= tcg_isa_2_07)
#define have_isa_3_00 (have_isa >= tcg_isa_3_00)
/* optional instructions automatically implemented */
@@ -155,7 +157,7 @@ extern bool have_vsx;
#define TCG_TARGET_HAS_v256 0
#define TCG_TARGET_HAS_andc_vec 1
-#define TCG_TARGET_HAS_orc_vec 0
+#define TCG_TARGET_HAS_orc_vec have_isa_2_07
#define TCG_TARGET_HAS_not_vec 1
#define TCG_TARGET_HAS_neg_vec 0
#define TCG_TARGET_HAS_abs_vec 0
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 2388958405..bc3a669cb4 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -484,6 +484,7 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VADDSWSVX4(896)
#define VADDUWSVX4(640)
#define VADDUWMVX4(128)
+#define VADDUDMVX4(192) /* v2.07 */
#define VSUBSBSVX4(1792)
#define VSUBUBSVX4(1536)
@@ -494,47 +495,62 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VSUBSWSVX4(1920)
#define VSUBUWSVX4(1664)
#define VSUBUWMVX4(1152)
+#define VSUBUDMVX4(1216) /* v2.07 */
#define VMAXSB VX4(258)
#define VMAXSH VX4(322)
#define VMAXSW VX4(386)
+#define VMAXSD VX4(450) /* v2.07 */
#define VMAXUB VX4(2)
#define VMAXUH VX4(66)
#define VMAXUW VX4(130)
+#define VMAXUD VX4(194) /* v2.07 */
#define VMINSB VX4(770)
#define VMINSH VX4(834)
#define VMINSW VX4(898)
+#define VMINSD VX4(962) /* v2.07 */
#define VMINUB VX4(514)
#define VMINUH VX4(578)
#define VMINUW VX4(642)
+#define VMINUD VX4(706) /* v2.07 */
#define VCMPEQUB VX4(6)
#define VCMPEQUH VX4(70)
#define VCMPEQUW VX4(134)
+#define VCMPEQUD VX4(199) /* v2.07 */
#define VCMPGTSB VX4(774)
#define VCMPGTSH VX4(838)
#define VCMPGTSW VX4(902)
+#define VCMPGTSD VX4(967) /* v2.07 */
#define VCMPGTUB VX4(518)
#define VCMPGTUH VX4(582)
#define VCMPGTUW VX4(646)
+#define VCMPGTUD VX4(711) /* v2.07 */
#define VSLB VX4(260)
#define VSLH VX4(324)
#define VSLW VX4(388)
+#define VSLD VX4(1476) /* v2.07 */
#define VSRB VX4(516)
#define VSRH VX4(580)
#define VSRW VX4(644)
+#define VSRD VX4(1732) /* v2.07 */
#define VSRAB VX4(772)
#define VSRAH VX4(836)
#define VSRAW VX4(900)
+#define VSRAD VX4(964) /* v2.07 */
#define VRLB VX4(4)
#define VRLH VX4(68)
#define VRLW VX4(132)
+#define VRLD VX4(196) /* v2.07 */
#define VMULEUBVX4(520)
#define VMULEUHVX4(584)
+#define VMULEUWVX4(648) /* v2.07 */
#define VMULOUBVX4(8)
#define VMULOUHVX4(72)
+#define VMULOUWVX4(136) /* v2.07 */
+#define VMULUWMVX4(137) /* v2.07 */
#define VMSUMUHM VX4(38)
#define VMRGHB VX4(12)
@@ -552,6 +568,9 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VNOR VX4(1284)
#define VORVX4(1156)
#define VXOR VX4(1220)
+#define VEQV VX4(1668) /* v2.07 */
+#define VNAND VX4(1412) /* v2.07 */
+#define VORC VX4(1348) /* v2.07 */
#define VSPLTB VX4(524)
#define VSPLTH VX4(588)
@@ -2904,26 +2923,37 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_andc_vec:
case INDEX_op_not_vec:
return 1;
+case INDEX_op_orc_vec:
+return have_isa_2_07;
case INDEX_op_add_vec:
case INDEX_op_sub_vec:
case INDEX_op_smax_vec:
case INDEX_op_smin_vec:
case INDEX_op_umax_vec:
case INDEX_op_umin_vec:
+case INDEX_op_shlv_vec:
+case INDEX_op_shrv_vec:
+case INDEX_op_sarv_vec:
+return vece <= MO_32 || have_isa_2_07;
case INDEX_op_ssadd_vec:
case INDEX_op_sssub_vec:
case INDEX_op_usadd_vec:
case INDEX_op_ussub_vec:
-case INDEX_op_shlv_vec:
-case INDEX_op_shrv_vec:
-case INDEX_op_sarv_vec:
return vece <= MO_32;
case INDEX_op_cmp_vec:
-case INDEX_op_mul_vec:
[PATCH v7 21/22] tcg/ppc: Update vector support for v3.00 load/store
These new instructions are a mix of those like LXSD that are
only conditional only on MSR.VEC and those like LXV that are
conditional on MSR.VEC for TX=1. Thus, in the end, we can
consider all of these as Altivec instructions.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 47
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index bd9259c60f..5b7d1bd2dc 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -471,11 +471,16 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define LXSDX (XO31(588) | 1) /* v2.06, force tx=1 */
#define LXVDSX (XO31(332) | 1) /* v2.06, force tx=1 */
#define LXSIWZX(XO31(12) | 1) /* v2.07, force tx=1 */
+#define LXV(OPCD(61) | 8 | 1) /* v3.00, force tx=1 */
+#define LXSD (OPCD(57) | 2) /* v3.00 */
+#define LXVWSX (XO31(364) | 1) /* v3.00, force tx=1 */
#define STVX XO31(231)
#define STVEWX XO31(199)
#define STXSDX (XO31(716) | 1) /* v2.06, force sx=1 */
#define STXSIWX(XO31(140) | 1) /* v2.07, force sx=1 */
+#define STXV (OPCD(61) | 8 | 5) /* v3.00, force sx=1 */
+#define STXSD (OPCD(61) | 2) /* v3.00 */
#define VADDSBSVX4(768)
#define VADDUBSVX4(512)
@@ -1114,7 +1119,7 @@ static void tcg_out_mem_long(TCGContext *s, int opi, int
opx, TCGReg rt,
TCGReg base, tcg_target_long offset)
{
tcg_target_long orig = offset, l0, l1, extra = 0, align = 0;
-bool is_store = false;
+bool is_int_store = false;
TCGReg rs = TCG_REG_TMP1;
switch (opi) {
@@ -1127,11 +1132,19 @@ static void tcg_out_mem_long(TCGContext *s, int opi,
int opx, TCGReg rt,
break;
}
break;
+case LXSD:
+case STXSD:
+align = 3;
+break;
+case LXV:
+case STXV:
+align = 15;
+break;
case STD:
align = 3;
/* FALLTHRU */
case STB: case STH: case STW:
-is_store = true;
+is_int_store = true;
break;
}
@@ -1140,7 +1153,7 @@ static void tcg_out_mem_long(TCGContext *s, int opi, int
opx, TCGReg rt,
if (rs == base) {
rs = TCG_REG_R0;
}
-tcg_debug_assert(!is_store || rs != rt);
+tcg_debug_assert(!is_int_store || rs != rt);
tcg_out_movi(s, TCG_TYPE_PTR, rs, orig);
tcg_out32(s, opx | TAB(rt & 31, base, rs));
return;
@@ -1205,7 +1218,8 @@ static void tcg_out_ld(TCGContext *s, TCGType type,
TCGReg ret,
case TCG_TYPE_V64:
tcg_debug_assert(ret >= TCG_REG_V0);
if (have_vsx) {
-tcg_out_mem_long(s, 0, LXSDX, ret, base, offset);
+tcg_out_mem_long(s, have_isa_3_00 ? LXSD : 0, LXSDX,
+ ret, base, offset);
break;
}
tcg_debug_assert((offset & 7) == 0);
@@ -1217,7 +1231,8 @@ static void tcg_out_ld(TCGContext *s, TCGType type,
TCGReg ret,
case TCG_TYPE_V128:
tcg_debug_assert(ret >= TCG_REG_V0);
tcg_debug_assert((offset & 15) == 0);
-tcg_out_mem_long(s, 0, LVX, ret, base, offset);
+tcg_out_mem_long(s, have_isa_3_00 ? LXV : 0,
+ LVX, ret, base, offset);
break;
default:
g_assert_not_reached();
@@ -1258,7 +1273,8 @@ static void tcg_out_st(TCGContext *s, TCGType type,
TCGReg arg,
case TCG_TYPE_V64:
tcg_debug_assert(arg >= TCG_REG_V0);
if (have_vsx) {
-tcg_out_mem_long(s, 0, STXSDX, arg, base, offset);
+tcg_out_mem_long(s, have_isa_3_00 ? STXSD : 0,
+ STXSDX, arg, base, offset);
break;
}
tcg_debug_assert((offset & 7) == 0);
@@ -1271,7 +1287,8 @@ static void tcg_out_st(TCGContext *s, TCGType type,
TCGReg arg,
break;
case TCG_TYPE_V128:
tcg_debug_assert(arg >= TCG_REG_V0);
-tcg_out_mem_long(s, 0, STVX, arg, base, offset);
+tcg_out_mem_long(s, have_isa_3_00 ? STXV : 0,
+ STVX, arg, base, offset);
break;
default:
g_assert_not_reached();
@@ -3042,7 +3059,11 @@ static bool tcg_out_dupm_vec(TCGContext *s, TCGType
type, unsigned vece,
tcg_debug_assert(out >= TCG_REG_V0);
switch (vece) {
case MO_8:
-tcg_out_mem_long(s, 0, LVEBX, out, base, offset);
+if (have_isa_3_00) {
+tcg_out_mem_long(s, LXV, LVX, out, base, offset & -16);
+} else {
+tcg_out_mem_long(s, 0, LVEBX, out, base, offset);
+}
elt = extract32(offset, 0, 4);
#ifndef HOST_WORDS_BIGENDIAN
elt ^= 15;
@@ -3051,7 +3072,11 @@ static bool tcg_out_dupm_vec(TCGContext *s, TCGType
type, unsigned vece,
break;
case MO_16:
tcg_debug_assert((offset & 1) == 0);
-tcg_out_mem_long(s, 0, LVEHX, out, base
[PATCH v7 15/22] tcg/ppc: Enable Altivec detection
Now that we have implemented the required tcg operations,
we can enable detection of host vector support.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 4
1 file changed, 4 insertions(+)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 8a508136ce..d739f4b605 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -3528,6 +3528,10 @@ static void tcg_target_init(TCGContext *s)
have_isel = have_isa_2_06;
#endif
+if (hwcap & PPC_FEATURE_HAS_ALTIVEC) {
+have_altivec = true;
+}
+
tcg_target_available_regs[TCG_TYPE_I32] = 0x;
tcg_target_available_regs[TCG_TYPE_I64] = 0x;
if (have_altivec) {
--
2.17.1
[PATCH v7 18/22] tcg/ppc: Update vector support for v2.07 VSX
These new instructions are conditional only on MSR.VSX and
are thus part of the VSX instruction set, and not Altivec.
This includes double-word loads and stores.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index bc3a669cb4..6321e0767f 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -470,10 +470,12 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define LVEWX XO31(71)
#define LXSDX (XO31(588) | 1) /* v2.06, force tx=1 */
#define LXVDSX (XO31(332) | 1) /* v2.06, force tx=1 */
+#define LXSIWZX(XO31(12) | 1) /* v2.07, force tx=1 */
#define STVX XO31(231)
#define STVEWX XO31(199)
#define STXSDX (XO31(716) | 1) /* v2.06, force sx=1 */
+#define STXSIWX(XO31(140) | 1) /* v2.07, force sx=1 */
#define VADDSBSVX4(768)
#define VADDUBSVX4(512)
@@ -1156,6 +1158,10 @@ static void tcg_out_ld(TCGContext *s, TCGType type,
TCGReg ret,
tcg_out_mem_long(s, LWZ, LWZX, ret, base, offset);
break;
}
+if (have_isa_2_07 && have_vsx) {
+tcg_out_mem_long(s, 0, LXSIWZX, ret, base, offset);
+break;
+}
tcg_debug_assert((offset & 3) == 0);
tcg_out_mem_long(s, 0, LVEWX, ret, base, offset);
shift = (offset - 4) & 0xc;
@@ -1203,6 +1209,11 @@ static void tcg_out_st(TCGContext *s, TCGType type,
TCGReg arg,
tcg_out_mem_long(s, STW, STWX, arg, base, offset);
break;
}
+if (have_isa_2_07 && have_vsx) {
+tcg_out_mem_long(s, 0, STXSIWX, arg, base, offset);
+break;
+}
+assert((offset & 3) == 0);
tcg_debug_assert((offset & 3) == 0);
shift = (offset - 4) & 0xc;
if (shift) {
--
2.17.1
[PATCH v5 06/10] audio: basic support for multichannel audio
Which currently only means removing some checks. Old code won't require
more than two channels, but new code will need it.
Signed-off-by: Kővágó, Zoltán
---
audio/alsaaudio.c | 7 ---
audio/audio.c | 2 +-
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index eddf013a53..f37ce1ce85 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -493,13 +493,6 @@ static int alsa_open(bool in, struct alsa_params_req *req,
goto err;
}
-if (nchannels != 1 && nchannels != 2) {
-alsa_logerr2 (err, typ,
- "Can not handle obtained number of channels %d\n",
- nchannels);
-goto err;
-}
-
if (apdo->buffer_length) {
int dir = 0;
unsigned int btime = apdo->buffer_length;
diff --git a/audio/audio.c b/audio/audio.c
index c00f4deddd..7fc3aa9d16 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -242,7 +242,7 @@ static int audio_validate_settings (struct audsettings *as)
{
int invalid;
-invalid = as->nchannels != 1 && as->nchannels != 2;
+invalid = as->nchannels < 1;
invalid |= as->endianness != 0 && as->endianness != 1;
switch (as->fmt) {
--
2.23.0
[PATCH v7 14/22] tcg/ppc: Support vector dup2
This is only used for 32-bit hosts. Signed-off-by: Richard Henderson Signed-off-by: Aleksandar Markovic --- tcg/ppc/tcg-target.inc.c | 9 + 1 file changed, 9 insertions(+) diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c index d4b3354626..8a508136ce 100644 --- a/tcg/ppc/tcg-target.inc.c +++ b/tcg/ppc/tcg-target.inc.c @@ -3102,6 +3102,14 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, } break; +case INDEX_op_dup2_vec: +assert(TCG_TARGET_REG_BITS == 32); +/* With inputs a1 = xLxx, a2 = xHxx */ +tcg_out32(s, VMRGHW | VRT(a0) | VRA(a2) | VRB(a1)); /* a0 = xxHL */ +tcg_out_vsldoi(s, TCG_VEC_TMP1, a0, a0, 8); /* tmp = HLxx */ +tcg_out_vsldoi(s, a0, a0, TCG_VEC_TMP1, 8); /* a0 = HLHL */ +return; + case INDEX_op_ppc_mrgh_vec: insn = mrgh_op[vece]; break; @@ -3480,6 +3488,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode op) case INDEX_op_ppc_mulou_vec: case INDEX_op_ppc_pkum_vec: case INDEX_op_ppc_rotl_vec: +case INDEX_op_dup2_vec: return &v_v_v; case INDEX_op_not_vec: case INDEX_op_dup_vec: -- 2.17.1
[PATCH v7 13/22] tcg/ppc: Support vector multiply
For Altivec, this is always an expansion.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 2 +-
tcg/ppc/tcg-target.opc.h | 8 +++
tcg/ppc/tcg-target.inc.c | 113 ++-
3 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index ffb226946b..f50b7f4bac 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -162,7 +162,7 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_shs_vec 0
#define TCG_TARGET_HAS_shv_vec 1
#define TCG_TARGET_HAS_cmp_vec 1
-#define TCG_TARGET_HAS_mul_vec 0
+#define TCG_TARGET_HAS_mul_vec 1
#define TCG_TARGET_HAS_sat_vec 1
#define TCG_TARGET_HAS_minmax_vec 1
#define TCG_TARGET_HAS_bitsel_vec 0
diff --git a/tcg/ppc/tcg-target.opc.h b/tcg/ppc/tcg-target.opc.h
index fa680dd6a0..db24a11987 100644
--- a/tcg/ppc/tcg-target.opc.h
+++ b/tcg/ppc/tcg-target.opc.h
@@ -3,3 +3,11 @@
* emitted by tcg_expand_vec_op. For those familiar with GCC internals,
* consider these to be UNSPEC with names.
*/
+
+DEF(ppc_mrgh_vec, 1, 2, 0, IMPLVEC)
+DEF(ppc_mrgl_vec, 1, 2, 0, IMPLVEC)
+DEF(ppc_msum_vec, 1, 3, 0, IMPLVEC)
+DEF(ppc_muleu_vec, 1, 2, 0, IMPLVEC)
+DEF(ppc_mulou_vec, 1, 2, 0, IMPLVEC)
+DEF(ppc_pkum_vec, 1, 2, 0, IMPLVEC)
+DEF(ppc_rotl_vec, 1, 2, 0, IMPLVEC)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index a9264eccbe..d4b3354626 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -523,6 +523,25 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VSRAB VX4(772)
#define VSRAH VX4(836)
#define VSRAW VX4(900)
+#define VRLB VX4(4)
+#define VRLH VX4(68)
+#define VRLW VX4(132)
+
+#define VMULEUBVX4(520)
+#define VMULEUHVX4(584)
+#define VMULOUBVX4(8)
+#define VMULOUHVX4(72)
+#define VMSUMUHM VX4(38)
+
+#define VMRGHB VX4(12)
+#define VMRGHH VX4(76)
+#define VMRGHW VX4(140)
+#define VMRGLB VX4(268)
+#define VMRGLH VX4(332)
+#define VMRGLW VX4(396)
+
+#define VPKUHUMVX4(14)
+#define VPKUWUMVX4(78)
#define VAND VX4(1028)
#define VANDC VX4(1092)
@@ -2875,6 +2894,7 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_sarv_vec:
return vece <= MO_32;
case INDEX_op_cmp_vec:
+case INDEX_op_mul_vec:
case INDEX_op_shli_vec:
case INDEX_op_shri_vec:
case INDEX_op_sari_vec:
@@ -2987,7 +3007,13 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
smax_op[4] = { VMAXSB, VMAXSH, VMAXSW, 0 },
shlv_op[4] = { VSLB, VSLH, VSLW, 0 },
shrv_op[4] = { VSRB, VSRH, VSRW, 0 },
-sarv_op[4] = { VSRAB, VSRAH, VSRAW, 0 };
+sarv_op[4] = { VSRAB, VSRAH, VSRAW, 0 },
+mrgh_op[4] = { VMRGHB, VMRGHH, VMRGHW, 0 },
+mrgl_op[4] = { VMRGLB, VMRGLH, VMRGLW, 0 },
+muleu_op[4] = { VMULEUB, VMULEUH, 0, 0 },
+mulou_op[4] = { VMULOUB, VMULOUH, 0, 0 },
+pkum_op[4] = { VPKUHUM, VPKUWUM, 0, 0 },
+rotl_op[4] = { VRLB, VRLH, VRLW, 0 };
TCGType type = vecl + TCG_TYPE_V64;
TCGArg a0 = args[0], a1 = args[1], a2 = args[2];
@@ -3076,6 +3102,29 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
}
break;
+case INDEX_op_ppc_mrgh_vec:
+insn = mrgh_op[vece];
+break;
+case INDEX_op_ppc_mrgl_vec:
+insn = mrgl_op[vece];
+break;
+case INDEX_op_ppc_muleu_vec:
+insn = muleu_op[vece];
+break;
+case INDEX_op_ppc_mulou_vec:
+insn = mulou_op[vece];
+break;
+case INDEX_op_ppc_pkum_vec:
+insn = pkum_op[vece];
+break;
+case INDEX_op_ppc_rotl_vec:
+insn = rotl_op[vece];
+break;
+case INDEX_op_ppc_msum_vec:
+tcg_debug_assert(vece == MO_16);
+tcg_out32(s, VMSUMUHM | VRT(a0) | VRA(a1) | VRB(a2) | VRC(args[3]));
+return;
+
case INDEX_op_mov_vec: /* Always emitted via tcg_out_mov. */
case INDEX_op_dupi_vec: /* Always emitted via tcg_out_movi. */
case INDEX_op_dup_vec: /* Always emitted via tcg_out_dup_vec. */
@@ -3145,6 +3194,53 @@ static void expand_vec_cmp(TCGType type, unsigned vece,
TCGv_vec v0,
}
}
+static void expand_vec_mul(TCGType type, unsigned vece, TCGv_vec v0,
+ TCGv_vec v1, TCGv_vec v2)
+{
+TCGv_vec t1 = tcg_temp_new_vec(type);
+TCGv_vec t2 = tcg_temp_new_vec(type);
+TCGv_vec t3, t4;
+
+switch (vece) {
+case MO_8:
+case MO_16:
+vec_gen_3(INDEX_op_ppc_muleu_vec, type, vece, tcgv_vec_arg(t1),
+ tcgv_vec_arg(v1), tcgv_vec_arg(v2));
+vec_gen_3(INDEX_op_ppc_mulou_vec, type, vece, tcgv_vec_arg(t2),
+ tcgv_vec_arg(v1), tcgv_vec_arg(v2));
+vec_gen_3(INDEX_op_ppc_mrgh_vec, type, vece + 1, tcgv_vec_arg
[PATCH v5 00/10] Audio: Mixeng-free 5.1/7.1 audio support
Hi, I've updated my mixeng-free audio patches. The documentation probably requires more polishing, but I'm sending this now so the other parts can be reviewed too. Changes from v4: * Removed already merged commits. * Bugfix in "audio: make mixeng optional" commit. * Dropped the "paaudio: channel-map option" patch, instead the paaudio backend will automatically figure out the current channel map for now (see the last patch). Currently this only works with usb-audio, if other frontends gain support for multi-channel audio, it might require some rethinking. Regards, Zoltan Kővágó, Zoltán (10): audio: add mixing-engine option (documentation) audio: make mixeng optional paaudio: get/put_buffer functions audio: support more than two channels in volume setting audio: replace shift in audio_pcm_info with bytes_per_frame audio: basic support for multichannel audio usb-audio: do not count on avail bytes actually available usb-audio: support more than two channels of audio usbaudio: change playback counters to 64 bit paaudio: fix channel order for usb-audio 5.1 and 7.1 streams audio/alsaaudio.c | 18 +- audio/audio.c | 176 ++- audio/audio.h | 10 + audio/audio_int.h | 7 +- audio/audio_template.h | 24 ++- audio/coreaudio.c | 4 +- audio/dsound_template.h | 10 +- audio/dsoundaudio.c | 4 +- audio/noaudio.c | 2 +- audio/ossaudio.c| 14 +- audio/paaudio.c | 153 -- audio/spiceaudio.c | 17 +- audio/wavaudio.c| 6 +- hw/usb/dev-audio.c | 459 ++-- qapi/audio.json | 5 + qemu-options.hx | 6 + 16 files changed, 730 insertions(+), 185 deletions(-) -- 2.23.0
[PATCH v7 07/22] tcg/ppc: Enable tcg backend vector compilation
Introduce all of the flags required to enable tcg backend vector support,
and a runtime flag to indicate the host supports Altivec instructions.
For now, do not actually set have_isa_altivec to true, because we have not
yet added all of the code to actually generate all of the required insns.
However, we must define these flags in order to disable ifndefs that create
stub versions of the functions added here.
The change to tcg_out_movi works around a buglet in tcg.c wherein if we
do not define tcg_out_dupi_vec we get a declared but not defined Werror,
but if we only declare it we get a defined but not used Werror. We need
to this change to tcg_out_movi eventually anyway, so it's no biggie.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 25
tcg/ppc/tcg-target.opc.h | 5
tcg/ppc/tcg-target.inc.c | 62 ++--
3 files changed, 89 insertions(+), 3 deletions(-)
create mode 100644 tcg/ppc/tcg-target.opc.h
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 35ba8693fa..498e950f0c 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -65,6 +65,7 @@ typedef enum {
} TCGPowerISA;
extern TCGPowerISA have_isa;
+extern bool have_altivec;
#define have_isa_2_06 (have_isa >= tcg_isa_2_06)
#define have_isa_3_00 (have_isa >= tcg_isa_3_00)
@@ -143,6 +144,30 @@ extern TCGPowerISA have_isa;
#define TCG_TARGET_HAS_mulsh_i641
#endif
+/*
+ * While technically Altivec could support V64, it has no 64-bit store
+ * instruction and substituting two 32-bit stores makes the generated
+ * code quite large.
+ */
+#define TCG_TARGET_HAS_v64 0
+#define TCG_TARGET_HAS_v128 have_altivec
+#define TCG_TARGET_HAS_v256 0
+
+#define TCG_TARGET_HAS_andc_vec 0
+#define TCG_TARGET_HAS_orc_vec 0
+#define TCG_TARGET_HAS_not_vec 0
+#define TCG_TARGET_HAS_neg_vec 0
+#define TCG_TARGET_HAS_abs_vec 0
+#define TCG_TARGET_HAS_shi_vec 0
+#define TCG_TARGET_HAS_shs_vec 0
+#define TCG_TARGET_HAS_shv_vec 0
+#define TCG_TARGET_HAS_cmp_vec 0
+#define TCG_TARGET_HAS_mul_vec 0
+#define TCG_TARGET_HAS_sat_vec 0
+#define TCG_TARGET_HAS_minmax_vec 0
+#define TCG_TARGET_HAS_bitsel_vec 0
+#define TCG_TARGET_HAS_cmpsel_vec 0
+
void flush_icache_range(uintptr_t start, uintptr_t stop);
void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t);
diff --git a/tcg/ppc/tcg-target.opc.h b/tcg/ppc/tcg-target.opc.h
new file mode 100644
index 00..fa680dd6a0
--- /dev/null
+++ b/tcg/ppc/tcg-target.opc.h
@@ -0,0 +1,5 @@
+/*
+ * Target-specific opcodes for host vector expansion. These will be
+ * emitted by tcg_expand_vec_op. For those familiar with GCC internals,
+ * consider these to be UNSPEC with names.
+ */
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index db28ae7eb1..c7ce0f923c 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -66,6 +66,7 @@ static tcg_insn_unit *tb_ret_addr;
TCGPowerISA have_isa;
static bool have_isel;
+bool have_altivec;
#ifndef CONFIG_SOFTMMU
#define TCG_GUEST_BASE_REG 30
@@ -714,10 +715,31 @@ static void tcg_out_movi_int(TCGContext *s, TCGType type,
TCGReg ret,
}
}
-static inline void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret,
-tcg_target_long arg)
+static void tcg_out_dupi_vec(TCGContext *s, TCGType type, TCGReg ret,
+ tcg_target_long val)
{
-tcg_out_movi_int(s, type, ret, arg, false);
+g_assert_not_reached();
+}
+
+static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret,
+ tcg_target_long arg)
+{
+switch (type) {
+case TCG_TYPE_I32:
+case TCG_TYPE_I64:
+tcg_debug_assert(ret < TCG_REG_V0);
+tcg_out_movi_int(s, type, ret, arg, false);
+break;
+
+case TCG_TYPE_V64:
+case TCG_TYPE_V128:
+tcg_debug_assert(ret >= TCG_REG_V0);
+tcg_out_dupi_vec(s, type, ret, arg);
+break;
+
+default:
+g_assert_not_reached();
+}
}
static bool mask_operand(uint32_t c, int *mb, int *me)
@@ -2602,6 +2624,36 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
const TCGArg *args,
}
}
+int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece)
+{
+g_assert_not_reached();
+}
+
+static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
+TCGReg dst, TCGReg src)
+{
+g_assert_not_reached();
+}
+
+static bool tcg_out_dupm_vec(TCGContext *s, TCGType type, unsigned vece,
+ TCGReg out, TCGReg base, intptr_t offset)
+{
+g_assert_not_reached();
+}
+
+static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
+ unsigned vecl, unsigned vece,
+ const TCGArg *args, cons
[PATCH v7 11/22] tcg/ppc: Add support for vector saturated add/subtract
Add support for vector saturated add/subtract using Altivec
instructions:
VADDSBS, VADDSHS, VADDSWS, VADDUBS, VADDUHS, VADDUWS, and
VSUBSBS, VSUBSHS, VSUBSWS, VSUBUBS, VSUBUHS, VSUBUWS.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 2 +-
tcg/ppc/tcg-target.inc.c | 36
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 13699f1b63..3ebbbfa77e 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -163,7 +163,7 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_shv_vec 0
#define TCG_TARGET_HAS_cmp_vec 1
#define TCG_TARGET_HAS_mul_vec 0
-#define TCG_TARGET_HAS_sat_vec 0
+#define TCG_TARGET_HAS_sat_vec 1
#define TCG_TARGET_HAS_minmax_vec 1
#define TCG_TARGET_HAS_bitsel_vec 0
#define TCG_TARGET_HAS_cmpsel_vec 0
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 6cfc78bb59..a1165209fc 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -471,12 +471,24 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define STVX XO31(231)
#define STVEWX XO31(199)
+#define VADDSBSVX4(768)
+#define VADDUBSVX4(512)
#define VADDUBMVX4(0)
+#define VADDSHSVX4(832)
+#define VADDUHSVX4(576)
#define VADDUHMVX4(64)
+#define VADDSWSVX4(896)
+#define VADDUWSVX4(640)
#define VADDUWMVX4(128)
+#define VSUBSBSVX4(1792)
+#define VSUBUBSVX4(1536)
#define VSUBUBMVX4(1024)
+#define VSUBSHSVX4(1856)
+#define VSUBUHSVX4(1600)
#define VSUBUHMVX4(1088)
+#define VSUBSWSVX4(1920)
+#define VSUBUWSVX4(1664)
#define VSUBUWMVX4(1152)
#define VMAXSB VX4(258)
@@ -2844,6 +2856,10 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_smin_vec:
case INDEX_op_umax_vec:
case INDEX_op_umin_vec:
+case INDEX_op_ssadd_vec:
+case INDEX_op_sssub_vec:
+case INDEX_op_usadd_vec:
+case INDEX_op_ussub_vec:
return vece <= MO_32;
case INDEX_op_cmp_vec:
return vece <= MO_32 ? -1 : 0;
@@ -2945,6 +2961,10 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
eq_op[4] = { VCMPEQUB, VCMPEQUH, VCMPEQUW, 0 },
gts_op[4] = { VCMPGTSB, VCMPGTSH, VCMPGTSW, 0 },
gtu_op[4] = { VCMPGTUB, VCMPGTUH, VCMPGTUW, 0 },
+ssadd_op[4] = { VADDSBS, VADDSHS, VADDSWS, 0 },
+usadd_op[4] = { VADDUBS, VADDUHS, VADDUWS, 0 },
+sssub_op[4] = { VSUBSBS, VSUBSHS, VSUBSWS, 0 },
+ussub_op[4] = { VSUBUBS, VSUBUHS, VSUBUWS, 0 },
umin_op[4] = { VMINUB, VMINUH, VMINUW, 0 },
smin_op[4] = { VMINSB, VMINSH, VMINSW, 0 },
umax_op[4] = { VMAXUB, VMAXUH, VMAXUW, 0 },
@@ -2971,6 +2991,18 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_sub_vec:
insn = sub_op[vece];
break;
+case INDEX_op_ssadd_vec:
+insn = ssadd_op[vece];
+break;
+case INDEX_op_sssub_vec:
+insn = sssub_op[vece];
+break;
+case INDEX_op_usadd_vec:
+insn = usadd_op[vece];
+break;
+case INDEX_op_ussub_vec:
+insn = ussub_op[vece];
+break;
case INDEX_op_smin_vec:
insn = smin_op[vece];
break;
@@ -3277,6 +3309,10 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
case INDEX_op_andc_vec:
case INDEX_op_orc_vec:
case INDEX_op_cmp_vec:
+case INDEX_op_ssadd_vec:
+case INDEX_op_sssub_vec:
+case INDEX_op_usadd_vec:
+case INDEX_op_ussub_vec:
case INDEX_op_smax_vec:
case INDEX_op_smin_vec:
case INDEX_op_umax_vec:
--
2.17.1
[PATCH v7 12/22] tcg/ppc: Support vector shift by immediate
For Altivec, this is done via vector shift by vector,
and loading the immediate into a register.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 2 +-
tcg/ppc/tcg-target.inc.c | 58 ++--
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 3ebbbfa77e..ffb226946b 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -160,7 +160,7 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_abs_vec 0
#define TCG_TARGET_HAS_shi_vec 0
#define TCG_TARGET_HAS_shs_vec 0
-#define TCG_TARGET_HAS_shv_vec 0
+#define TCG_TARGET_HAS_shv_vec 1
#define TCG_TARGET_HAS_cmp_vec 1
#define TCG_TARGET_HAS_mul_vec 0
#define TCG_TARGET_HAS_sat_vec 1
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index a1165209fc..a9264eccbe 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -514,6 +514,16 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VCMPGTUH VX4(582)
#define VCMPGTUW VX4(646)
+#define VSLB VX4(260)
+#define VSLH VX4(324)
+#define VSLW VX4(388)
+#define VSRB VX4(516)
+#define VSRH VX4(580)
+#define VSRW VX4(644)
+#define VSRAB VX4(772)
+#define VSRAH VX4(836)
+#define VSRAW VX4(900)
+
#define VAND VX4(1028)
#define VANDC VX4(1092)
#define VNOR VX4(1284)
@@ -2860,8 +2870,14 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_sssub_vec:
case INDEX_op_usadd_vec:
case INDEX_op_ussub_vec:
+case INDEX_op_shlv_vec:
+case INDEX_op_shrv_vec:
+case INDEX_op_sarv_vec:
return vece <= MO_32;
case INDEX_op_cmp_vec:
+case INDEX_op_shli_vec:
+case INDEX_op_shri_vec:
+case INDEX_op_sari_vec:
return vece <= MO_32 ? -1 : 0;
default:
return 0;
@@ -2968,7 +2984,10 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
umin_op[4] = { VMINUB, VMINUH, VMINUW, 0 },
smin_op[4] = { VMINSB, VMINSH, VMINSW, 0 },
umax_op[4] = { VMAXUB, VMAXUH, VMAXUW, 0 },
-smax_op[4] = { VMAXSB, VMAXSH, VMAXSW, 0 };
+smax_op[4] = { VMAXSB, VMAXSH, VMAXSW, 0 },
+shlv_op[4] = { VSLB, VSLH, VSLW, 0 },
+shrv_op[4] = { VSRB, VSRH, VSRW, 0 },
+sarv_op[4] = { VSRAB, VSRAH, VSRAW, 0 };
TCGType type = vecl + TCG_TYPE_V64;
TCGArg a0 = args[0], a1 = args[1], a2 = args[2];
@@ -3015,6 +3034,15 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_umax_vec:
insn = umax_op[vece];
break;
+case INDEX_op_shlv_vec:
+insn = shlv_op[vece];
+break;
+case INDEX_op_shrv_vec:
+insn = shrv_op[vece];
+break;
+case INDEX_op_sarv_vec:
+insn = sarv_op[vece];
+break;
case INDEX_op_and_vec:
insn = VAND;
break;
@@ -3059,6 +3087,18 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
tcg_out32(s, insn | VRT(a0) | VRA(a1) | VRB(a2));
}
+static void expand_vec_shi(TCGType type, unsigned vece, TCGv_vec v0,
+ TCGv_vec v1, TCGArg imm, TCGOpcode opci)
+{
+TCGv_vec t1 = tcg_temp_new_vec(type);
+
+/* Splat w/bytes for xxspltib. */
+tcg_gen_dupi_vec(MO_8, t1, imm & ((8 << vece) - 1));
+vec_gen_3(opci, type, vece, tcgv_vec_arg(v0),
+ tcgv_vec_arg(v1), tcgv_vec_arg(t1));
+tcg_temp_free_vec(t1);
+}
+
static void expand_vec_cmp(TCGType type, unsigned vece, TCGv_vec v0,
TCGv_vec v1, TCGv_vec v2, TCGCond cond)
{
@@ -3110,14 +3150,25 @@ void tcg_expand_vec_op(TCGOpcode opc, TCGType type,
unsigned vece,
{
va_list va;
TCGv_vec v0, v1, v2;
+TCGArg a2;
va_start(va, a0);
v0 = temp_tcgv_vec(arg_temp(a0));
v1 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
-v2 = temp_tcgv_vec(arg_temp(va_arg(va, TCGArg)));
+a2 = va_arg(va, TCGArg);
switch (opc) {
+case INDEX_op_shli_vec:
+expand_vec_shi(type, vece, v0, v1, a2, INDEX_op_shlv_vec);
+break;
+case INDEX_op_shri_vec:
+expand_vec_shi(type, vece, v0, v1, a2, INDEX_op_shrv_vec);
+break;
+case INDEX_op_sari_vec:
+expand_vec_shi(type, vece, v0, v1, a2, INDEX_op_sarv_vec);
+break;
case INDEX_op_cmp_vec:
+v2 = temp_tcgv_vec(arg_temp(a2));
expand_vec_cmp(type, vece, v0, v1, v2, va_arg(va, TCGArg));
break;
default:
@@ -3317,6 +3368,9 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
case INDEX_op_smin_vec:
case INDEX_op_umax_vec:
case INDEX_op_umin_vec:
+case INDEX_op_shlv_vec:
+case INDEX_op_shrv_vec:
+case INDEX_op_sarv_vec:
return &v_v_v;
case INDEX_op_not_vec:
case INDEX_op_dup_vec:
--
2.17.1
[PATCH v7 05/22] tcg/ppc: Replace HAVE_ISA_2_06
This is identical to have_isa_2_06, so replace it.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 0bfaef9418..7cb0002c14 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -66,7 +66,6 @@ static tcg_insn_unit *tb_ret_addr;
TCGPowerISA have_isa;
-#define HAVE_ISA_2_06 have_isa_2_06
#define HAVE_ISEL have_isa_2_06
#ifndef CONFIG_SOFTMMU
@@ -1797,7 +1796,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg
*args, bool is_64)
}
} else {
uint32_t insn = qemu_ldx_opc[opc & (MO_BSWAP | MO_SSIZE)];
-if (!HAVE_ISA_2_06 && insn == LDBRX) {
+if (!have_isa_2_06 && insn == LDBRX) {
tcg_out32(s, ADDI | TAI(TCG_REG_R0, addrlo, 4));
tcg_out32(s, LWBRX | TAB(datalo, rbase, addrlo));
tcg_out32(s, LWBRX | TAB(TCG_REG_R0, rbase, TCG_REG_R0));
@@ -1869,7 +1868,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg
*args, bool is_64)
}
} else {
uint32_t insn = qemu_stx_opc[opc & (MO_BSWAP | MO_SIZE)];
-if (!HAVE_ISA_2_06 && insn == STDBRX) {
+if (!have_isa_2_06 && insn == STDBRX) {
tcg_out32(s, STWBRX | SAB(datalo, rbase, addrlo));
tcg_out32(s, ADDI | TAI(TCG_REG_TMP1, addrlo, 4));
tcg_out_shri64(s, TCG_REG_R0, datalo, 32);
--
2.17.1
[PATCH v7 20/22] tcg/ppc: Update vector support for v3.00 Altivec
These new instructions are conditional only on MSR.VEC and
are thus part of the Altivec instruction set, and not VSX.
This includes negation and compare not equal.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.h | 2 +-
tcg/ppc/tcg-target.inc.c | 23 +++
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 13197eddce..4fa21f0e71 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -159,7 +159,7 @@ extern bool have_vsx;
#define TCG_TARGET_HAS_andc_vec 1
#define TCG_TARGET_HAS_orc_vec have_isa_2_07
#define TCG_TARGET_HAS_not_vec 1
-#define TCG_TARGET_HAS_neg_vec 0
+#define TCG_TARGET_HAS_neg_vec have_isa_3_00
#define TCG_TARGET_HAS_abs_vec 0
#define TCG_TARGET_HAS_shi_vec 0
#define TCG_TARGET_HAS_shs_vec 0
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 840464aab5..bd9259c60f 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -499,6 +499,9 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VSUBUWMVX4(1152)
#define VSUBUDMVX4(1216) /* v2.07 */
+#define VNEGW (VX4(1538) | (6 << 16)) /* v3.00 */
+#define VNEGD (VX4(1538) | (7 << 16)) /* v3.00 */
+
#define VMAXSB VX4(258)
#define VMAXSH VX4(322)
#define VMAXSW VX4(386)
@@ -528,6 +531,9 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VCMPGTUH VX4(582)
#define VCMPGTUW VX4(646)
#define VCMPGTUD VX4(711) /* v2.07 */
+#define VCMPNEBVX4(7) /* v3.00 */
+#define VCMPNEHVX4(71)/* v3.00 */
+#define VCMPNEWVX4(135) /* v3.00 */
#define VSLB VX4(260)
#define VSLH VX4(324)
@@ -2976,6 +2982,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_shri_vec:
case INDEX_op_sari_vec:
return vece <= MO_32 || have_isa_2_07 ? -1 : 0;
+case INDEX_op_neg_vec:
+return vece >= MO_32 && have_isa_3_00;
case INDEX_op_mul_vec:
switch (vece) {
case MO_8:
@@ -3090,7 +3098,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
static const uint32_t
add_op[4] = { VADDUBM, VADDUHM, VADDUWM, VADDUDM },
sub_op[4] = { VSUBUBM, VSUBUHM, VSUBUWM, VSUBUDM },
+neg_op[4] = { 0, 0, VNEGW, VNEGD },
eq_op[4] = { VCMPEQUB, VCMPEQUH, VCMPEQUW, VCMPEQUD },
+ne_op[4] = { VCMPNEB, VCMPNEH, VCMPNEW, 0 },
gts_op[4] = { VCMPGTSB, VCMPGTSH, VCMPGTSW, VCMPGTSD },
gtu_op[4] = { VCMPGTUB, VCMPGTUH, VCMPGTUW, VCMPGTUD },
ssadd_op[4] = { VADDSBS, VADDSHS, VADDSWS, 0 },
@@ -3132,6 +3142,11 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
case INDEX_op_sub_vec:
insn = sub_op[vece];
break;
+case INDEX_op_neg_vec:
+insn = neg_op[vece];
+a2 = a1;
+a1 = 0;
+break;
case INDEX_op_mul_vec:
tcg_debug_assert(vece == MO_32 && have_isa_2_07);
insn = VMULUWM;
@@ -3194,6 +3209,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
case TCG_COND_EQ:
insn = eq_op[vece];
break;
+case TCG_COND_NE:
+insn = ne_op[vece];
+break;
case TCG_COND_GT:
insn = gts_op[vece];
break;
@@ -3276,6 +3294,10 @@ static void expand_vec_cmp(TCGType type, unsigned vece,
TCGv_vec v0,
case TCG_COND_GTU:
break;
case TCG_COND_NE:
+if (have_isa_3_00 && vece <= MO_32) {
+break;
+}
+/* fall through */
case TCG_COND_LE:
case TCG_COND_LEU:
need_inv = true;
@@ -3599,6 +3621,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
case INDEX_op_dup2_vec:
return &v_v_v;
case INDEX_op_not_vec:
+case INDEX_op_neg_vec:
case INDEX_op_dup_vec:
return &v_v;
case INDEX_op_ld_vec:
--
2.17.1
[PATCH v7 08/22] tcg/ppc: Add support for load/store/logic/comparison
Add various bits and peaces related mostly to load and store
operations. In that context, logic, compare, and splat Altivec
instructions are used, and, therefore, the support for emitting
them is included in this patch too.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 6 +-
tcg/ppc/tcg-target.inc.c | 472 ---
2 files changed, 442 insertions(+), 36 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 498e950f0c..a0e59a5074 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -153,15 +153,15 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_v128 have_altivec
#define TCG_TARGET_HAS_v256 0
-#define TCG_TARGET_HAS_andc_vec 0
+#define TCG_TARGET_HAS_andc_vec 1
#define TCG_TARGET_HAS_orc_vec 0
-#define TCG_TARGET_HAS_not_vec 0
+#define TCG_TARGET_HAS_not_vec 1
#define TCG_TARGET_HAS_neg_vec 0
#define TCG_TARGET_HAS_abs_vec 0
#define TCG_TARGET_HAS_shi_vec 0
#define TCG_TARGET_HAS_shs_vec 0
#define TCG_TARGET_HAS_shv_vec 0
-#define TCG_TARGET_HAS_cmp_vec 0
+#define TCG_TARGET_HAS_cmp_vec 1
#define TCG_TARGET_HAS_mul_vec 0
#define TCG_TARGET_HAS_sat_vec 0
#define TCG_TARGET_HAS_minmax_vec 0
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index c7ce0f923c..1a8d7dc925 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -230,6 +230,10 @@ static const char
*target_parse_constraint(TCGArgConstraint *ct,
ct->ct |= TCG_CT_REG;
ct->u.regs = 0x;
break;
+case 'v':
+ct->ct |= TCG_CT_REG;
+ct->u.regs = 0xull;
+break;
case 'L': /* qemu_ld constraint */
ct->ct |= TCG_CT_REG;
ct->u.regs = 0x;
@@ -459,6 +463,39 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define NOPORI /* ori 0,0,0 */
+#define LVXXO31(103)
+#define LVEBX XO31(7)
+#define LVEHX XO31(39)
+#define LVEWX XO31(71)
+
+#define STVX XO31(231)
+#define STVEWX XO31(199)
+
+#define VCMPEQUB VX4(6)
+#define VCMPEQUH VX4(70)
+#define VCMPEQUW VX4(134)
+#define VCMPGTSB VX4(774)
+#define VCMPGTSH VX4(838)
+#define VCMPGTSW VX4(902)
+#define VCMPGTUB VX4(518)
+#define VCMPGTUH VX4(582)
+#define VCMPGTUW VX4(646)
+
+#define VAND VX4(1028)
+#define VANDC VX4(1092)
+#define VNOR VX4(1284)
+#define VORVX4(1156)
+#define VXOR VX4(1220)
+
+#define VSPLTB VX4(524)
+#define VSPLTH VX4(588)
+#define VSPLTW VX4(652)
+#define VSPLTISB VX4(780)
+#define VSPLTISH VX4(844)
+#define VSPLTISW VX4(908)
+
+#define VSLDOI VX4(44)
+
#define RT(r) ((r)<<21)
#define RS(r) ((r)<<21)
#define RA(r) ((r)<<16)
@@ -532,6 +569,8 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
intptr_t value, intptr_t addend)
{
tcg_insn_unit *target;
+int16_t lo;
+int32_t hi;
value += addend;
target = (tcg_insn_unit *)value;
@@ -553,6 +592,20 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
}
*code_ptr = (*code_ptr & ~0xfffc) | (value & 0xfffc);
break;
+case R_PPC_ADDR32:
+/*
+ * We are abusing this relocation type. Again, this points to
+ * a pair of insns, lis + load. This is an absolute address
+ * relocation for PPC32 so the lis cannot be removed.
+ */
+lo = value;
+hi = value - lo;
+if (hi + lo != value) {
+return false;
+}
+code_ptr[0] = deposit32(code_ptr[0], 0, 16, hi >> 16);
+code_ptr[1] = deposit32(code_ptr[1], 0, 16, lo);
+break;
default:
g_assert_not_reached();
}
@@ -564,9 +617,29 @@ static void tcg_out_mem_long(TCGContext *s, int opi, int
opx, TCGReg rt,
static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
{
-tcg_debug_assert(TCG_TARGET_REG_BITS == 64 || type == TCG_TYPE_I32);
-if (ret != arg) {
-tcg_out32(s, OR | SAB(arg, ret, arg));
+if (ret == arg) {
+return true;
+}
+switch (type) {
+case TCG_TYPE_I64:
+tcg_debug_assert(TCG_TARGET_REG_BITS == 64);
+/* fallthru */
+case TCG_TYPE_I32:
+if (ret < TCG_REG_V0 && arg < TCG_REG_V0) {
+tcg_out32(s, OR | SAB(arg, ret, arg));
+break;
+} else if (ret < TCG_REG_V0 || arg < TCG_REG_V0) {
+/* Altivec does not support vector/integer moves. */
+return false;
+}
+/* fallthru */
+case TCG_TYPE_V64:
+case TCG_TYPE_V128:
+tcg_debug_assert(ret >= TCG_REG_V0 && arg >= TCG_REG_V0);
+tcg_out32(s, VOR | VRT(ret) | VRA(arg) | VRB(arg));
+break;
+de
[PATCH v7 10/22] tcg/ppc: Add support for vector add/subtract
Add support for vector add/subtract using Altivec instructions:
VADDUBM, VADDUHM, VADDUWM, VSUBUBM, VSUBUHM, VSUBUWM.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.inc.c | 20
1 file changed, 20 insertions(+)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 6879be6f80..6cfc78bb59 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -471,6 +471,14 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define STVX XO31(231)
#define STVEWX XO31(199)
+#define VADDUBMVX4(0)
+#define VADDUHMVX4(64)
+#define VADDUWMVX4(128)
+
+#define VSUBUBMVX4(1024)
+#define VSUBUHMVX4(1088)
+#define VSUBUWMVX4(1152)
+
#define VMAXSB VX4(258)
#define VMAXSH VX4(322)
#define VMAXSW VX4(386)
@@ -2830,6 +2838,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_andc_vec:
case INDEX_op_not_vec:
return 1;
+case INDEX_op_add_vec:
+case INDEX_op_sub_vec:
case INDEX_op_smax_vec:
case INDEX_op_smin_vec:
case INDEX_op_umax_vec:
@@ -2930,6 +2940,8 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
const TCGArg *args, const int *const_args)
{
static const uint32_t
+add_op[4] = { VADDUBM, VADDUHM, VADDUWM, 0 },
+sub_op[4] = { VSUBUBM, VSUBUHM, VSUBUWM, 0 },
eq_op[4] = { VCMPEQUB, VCMPEQUH, VCMPEQUW, 0 },
gts_op[4] = { VCMPGTSB, VCMPGTSH, VCMPGTSW, 0 },
gtu_op[4] = { VCMPGTUB, VCMPGTUH, VCMPGTUW, 0 },
@@ -2953,6 +2965,12 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
tcg_out_dupm_vec(s, type, vece, a0, a1, a2);
return;
+case INDEX_op_add_vec:
+insn = add_op[vece];
+break;
+case INDEX_op_sub_vec:
+insn = sub_op[vece];
+break;
case INDEX_op_smin_vec:
insn = smin_op[vece];
break;
@@ -3251,6 +3269,8 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
return (TCG_TARGET_REG_BITS == 64 ? &S_S
: TARGET_LONG_BITS == 32 ? &S_S_S : &S_S_S_S);
+case INDEX_op_add_vec:
+case INDEX_op_sub_vec:
case INDEX_op_and_vec:
case INDEX_op_or_vec:
case INDEX_op_xor_vec:
--
2.17.1
[PATCH v7 04/22] tcg/ppc: Create TCGPowerISA and have_isa
Introduce an enum to hold base < 2.06 < 3.00. Use macros to
preserve the existing have_isa_2_06 and have_isa_3_00 predicates.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.h | 12 ++--
tcg/ppc/tcg-target.inc.c | 8
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 690fa744e1..35ba8693fa 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -58,8 +58,16 @@ typedef enum {
TCG_AREG0 = TCG_REG_R27
} TCGReg;
-extern bool have_isa_2_06;
-extern bool have_isa_3_00;
+typedef enum {
+tcg_isa_base,
+tcg_isa_2_06,
+tcg_isa_3_00,
+} TCGPowerISA;
+
+extern TCGPowerISA have_isa;
+
+#define have_isa_2_06 (have_isa >= tcg_isa_2_06)
+#define have_isa_3_00 (have_isa >= tcg_isa_3_00)
/* optional instructions automatically implemented */
#define TCG_TARGET_HAS_ext8u_i320 /* andi */
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 4aad5d2b36..0bfaef9418 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -64,8 +64,7 @@
static tcg_insn_unit *tb_ret_addr;
-bool have_isa_2_06;
-bool have_isa_3_00;
+TCGPowerISA have_isa;
#define HAVE_ISA_2_06 have_isa_2_06
#define HAVE_ISEL have_isa_2_06
@@ -2787,12 +2786,13 @@ static void tcg_target_init(TCGContext *s)
unsigned long hwcap = qemu_getauxval(AT_HWCAP);
unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2);
+have_isa = tcg_isa_base;
if (hwcap & PPC_FEATURE_ARCH_2_06) {
-have_isa_2_06 = true;
+have_isa = tcg_isa_2_06;
}
#ifdef PPC_FEATURE2_ARCH_3_00
if (hwcap2 & PPC_FEATURE2_ARCH_3_00) {
-have_isa_3_00 = true;
+have_isa = tcg_isa_3_00;
}
#endif
--
2.17.1
[PATCH v7 03/22] tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC()
Introduce macros VRT(), VRA(), VRB(), VRC() used for encoding elements of Altivec instructions. Signed-off-by: Richard Henderson Signed-off-by: Aleksandar Markovic --- tcg/ppc/tcg-target.inc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c index 8dc5455600..4aad5d2b36 100644 --- a/tcg/ppc/tcg-target.inc.c +++ b/tcg/ppc/tcg-target.inc.c @@ -473,6 +473,11 @@ static int tcg_target_const_match(tcg_target_long val, TCGType type, #define MB64(b) ((b)<<5) #define FXM(b) (1 << (19 - (b))) +#define VRT(r) (((r) & 31) << 21) +#define VRA(r) (((r) & 31) << 16) +#define VRB(r) (((r) & 31) << 11) +#define VRC(r) (((r) & 31) << 6) + #define LK1 #define TAB(t, a, b) (RT(t) | RA(a) | RB(b)) -- 2.17.1
[PATCH v7 19/22] tcg/ppc: Update vector support for v2.07 FP
These new instructions are conditional on MSR.FP when TX=0 and
MSR.VEC when TX=1. Since we only care about the Altivec registers,
and force TX=1, we can consider these to be Altivec instructions.
Since Altivec is true for any use of vector types, we only need
test have_isa_2_07.
This includes moves to and from the integer registers.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 6321e0767f..840464aab5 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -586,6 +586,11 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define XXPERMDI (OPCD(60) | (10 << 3) | 7) /* v2.06, force ax=bx=tx=1 */
#define XXSEL (OPCD(60) | (3 << 4) | 0xf) /* v2.06, force ax=bx=cx=tx=1 */
+#define MFVSRD (XO31(51) | 1) /* v2.07, force sx=1 */
+#define MFVSRWZ(XO31(115) | 1) /* v2.07, force sx=1 */
+#define MTVSRD (XO31(179) | 1) /* v2.07, force tx=1 */
+#define MTVSRWZ(XO31(243) | 1) /* v2.07, force tx=1 */
+
#define RT(r) ((r)<<21)
#define RS(r) ((r)<<21)
#define RA(r) ((r)<<16)
@@ -715,12 +720,27 @@ static bool tcg_out_mov(TCGContext *s, TCGType type,
TCGReg ret, TCGReg arg)
tcg_debug_assert(TCG_TARGET_REG_BITS == 64);
/* fallthru */
case TCG_TYPE_I32:
-if (ret < TCG_REG_V0 && arg < TCG_REG_V0) {
-tcg_out32(s, OR | SAB(arg, ret, arg));
-break;
-} else if (ret < TCG_REG_V0 || arg < TCG_REG_V0) {
-/* Altivec does not support vector/integer moves. */
-return false;
+if (ret < TCG_REG_V0) {
+if (arg < TCG_REG_V0) {
+tcg_out32(s, OR | SAB(arg, ret, arg));
+break;
+} else if (have_isa_2_07) {
+tcg_out32(s, (type == TCG_TYPE_I32 ? MFVSRWZ : MFVSRD)
+ | VRT(arg) | RA(ret));
+break;
+} else {
+/* Altivec does not support vector->integer moves. */
+return false;
+}
+} else if (arg < TCG_REG_V0) {
+if (have_isa_2_07) {
+tcg_out32(s, (type == TCG_TYPE_I32 ? MTVSRWZ : MTVSRD)
+ | VRT(ret) | RA(arg));
+break;
+} else {
+/* Altivec does not support integer->vector moves. */
+return false;
+}
}
/* fallthru */
case TCG_TYPE_V64:
--
2.17.1
[PATCH v7 09/22] tcg/ppc: Add support for vector maximum/minimum
Add support for vector maximum/minimum using Altivec instructions
VMAXSB, VMAXSH, VMAXSW, VMAXUB, VMAXUH, VMAXUW, and
VMINSB, VMINSH, VMINSW, VMINUB, VMINUH, VMINUW.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 2 +-
tcg/ppc/tcg-target.inc.c | 40 +++-
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index a0e59a5074..13699f1b63 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -164,7 +164,7 @@ extern bool have_altivec;
#define TCG_TARGET_HAS_cmp_vec 1
#define TCG_TARGET_HAS_mul_vec 0
#define TCG_TARGET_HAS_sat_vec 0
-#define TCG_TARGET_HAS_minmax_vec 0
+#define TCG_TARGET_HAS_minmax_vec 1
#define TCG_TARGET_HAS_bitsel_vec 0
#define TCG_TARGET_HAS_cmpsel_vec 0
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 1a8d7dc925..6879be6f80 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -471,6 +471,19 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define STVX XO31(231)
#define STVEWX XO31(199)
+#define VMAXSB VX4(258)
+#define VMAXSH VX4(322)
+#define VMAXSW VX4(386)
+#define VMAXUB VX4(2)
+#define VMAXUH VX4(66)
+#define VMAXUW VX4(130)
+#define VMINSB VX4(770)
+#define VMINSH VX4(834)
+#define VMINSW VX4(898)
+#define VMINUB VX4(514)
+#define VMINUH VX4(578)
+#define VMINUW VX4(642)
+
#define VCMPEQUB VX4(6)
#define VCMPEQUH VX4(70)
#define VCMPEQUW VX4(134)
@@ -2817,6 +2830,11 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
case INDEX_op_andc_vec:
case INDEX_op_not_vec:
return 1;
+case INDEX_op_smax_vec:
+case INDEX_op_smin_vec:
+case INDEX_op_umax_vec:
+case INDEX_op_umin_vec:
+return vece <= MO_32;
case INDEX_op_cmp_vec:
return vece <= MO_32 ? -1 : 0;
default:
@@ -2914,7 +2932,11 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
static const uint32_t
eq_op[4] = { VCMPEQUB, VCMPEQUH, VCMPEQUW, 0 },
gts_op[4] = { VCMPGTSB, VCMPGTSH, VCMPGTSW, 0 },
-gtu_op[4] = { VCMPGTUB, VCMPGTUH, VCMPGTUW, 0 };
+gtu_op[4] = { VCMPGTUB, VCMPGTUH, VCMPGTUW, 0 },
+umin_op[4] = { VMINUB, VMINUH, VMINUW, 0 },
+smin_op[4] = { VMINSB, VMINSH, VMINSW, 0 },
+umax_op[4] = { VMAXUB, VMAXUH, VMAXUW, 0 },
+smax_op[4] = { VMAXSB, VMAXSH, VMAXSW, 0 };
TCGType type = vecl + TCG_TYPE_V64;
TCGArg a0 = args[0], a1 = args[1], a2 = args[2];
@@ -2931,6 +2953,18 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
tcg_out_dupm_vec(s, type, vece, a0, a1, a2);
return;
+case INDEX_op_smin_vec:
+insn = smin_op[vece];
+break;
+case INDEX_op_umin_vec:
+insn = umin_op[vece];
+break;
+case INDEX_op_smax_vec:
+insn = smax_op[vece];
+break;
+case INDEX_op_umax_vec:
+insn = umax_op[vece];
+break;
case INDEX_op_and_vec:
insn = VAND;
break;
@@ -3223,6 +3257,10 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode
op)
case INDEX_op_andc_vec:
case INDEX_op_orc_vec:
case INDEX_op_cmp_vec:
+case INDEX_op_smax_vec:
+case INDEX_op_smin_vec:
+case INDEX_op_umax_vec:
+case INDEX_op_umin_vec:
return &v_v_v;
case INDEX_op_not_vec:
case INDEX_op_dup_vec:
--
2.17.1
[PATCH v7 02/22] tcg/ppc: Introduce macro VX4()
Introduce macro VX4() used for encoding Altivec instructions. Signed-off-by: Richard Henderson Signed-off-by: Aleksandar Markovic --- tcg/ppc/tcg-target.inc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c index 9d678c3bf1..8dc5455600 100644 --- a/tcg/ppc/tcg-target.inc.c +++ b/tcg/ppc/tcg-target.inc.c @@ -319,6 +319,7 @@ static int tcg_target_const_match(tcg_target_long val, TCGType type, #define XO31(opc) (OPCD(31)|((opc)<<1)) #define XO58(opc) (OPCD(58)|(opc)) #define XO62(opc) (OPCD(62)|(opc)) +#define VX4(opc) (OPCD(4)|(opc)) #define B OPCD( 18) #define BC OPCD( 16) -- 2.17.1
[PATCH v7 00/22] tcg/ppc: Add vector opcodes
Changes since v6:
* The have_foo tests have been split so that VSX is not
combined with ISA revision.
* The power{7,8,9} patches have been split by isa extension.
* Force the [TABC]X bits on within the VSX instruction defines,
making the usage of the VSX insns clearer, since we have no
additional or'ing of seemingly random bits.
Changes since v5:
* Disable runtime altivec detection until all of the required
opcodes are implemented.
Because dup2 was last, that really means all of the pure altivec
bits, so the initial patches are not bisectable in any meaningful
sense. I thought about reshuffling dup2 earlier, but that created
too many conflicts and I was too lazy.
* Rearranged the patches a little bit to make sure that each
one actually builds, which was not the case before.
* Folded in the fix to tcg_out_mem_long, as discussed in the
followup within the v4 thread.
Changes since v4:
* Patch 1, "tcg/ppc: Introduce Altivec registers", is divided into
ten smaller patches.
* The net result (code-wise) is not changed between former patch 1
and ten new patches.
* Remaining (2-7) patches from v4 are applied verbatim.
* This means that code-wise v5 and v4 do not differ.
* v5 is devised to help debugging, and to better organize the code.
Changes since v3:
* Add support for bitsel, with the vsx xxsel insn.
* Rely on the new relocation overflow handling, so
we don't require 3 insns for a vector load.
Changes since v2:
* Several generic tcg patches to improve dup vs dupi vs dupm.
In particular, if a global temp (like guest r10) is not in
a host register, we should duplicate from memory instead of
loading to an integer register, spilling to stack, loading
to a vector register, and then duplicating.
* I have more confidence that 32-bit ppc host should work
this time around. No testing on that front yet, but I've
unified some code sequences with 64-bit ppc host.
* Base altivec now supports V128 only. Moved V64 support to
Power7 (v2.06), which has 64-bit load/store.
* Dropped support for 64-bit vector multiply using Power8.
The expansion was too large compared to using integer regs.
Richard Henderson (22):
tcg/ppc: Introduce Altivec registers
tcg/ppc: Introduce macro VX4()
tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC()
tcg/ppc: Create TCGPowerISA and have_isa
tcg/ppc: Replace HAVE_ISA_2_06
tcg/ppc: Replace HAVE_ISEL macro with a variable
tcg/ppc: Enable tcg backend vector compilation
tcg/ppc: Add support for load/store/logic/comparison
tcg/ppc: Add support for vector maximum/minimum
tcg/ppc: Add support for vector add/subtract
tcg/ppc: Add support for vector saturated add/subtract
tcg/ppc: Support vector shift by immediate
tcg/ppc: Support vector multiply
tcg/ppc: Support vector dup2
tcg/ppc: Enable Altivec detection
tcg/ppc: Update vector support for VSX
tcg/ppc: Update vector support for v2.07 Altivec
tcg/ppc: Update vector support for v2.07 VSX
tcg/ppc: Update vector support for v2.07 FP
tcg/ppc: Update vector support for v3.00 Altivec
tcg/ppc: Update vector support for v3.00 load/store
tcg/ppc: Update vector support for v3.00 dup/dupi
tcg/ppc/tcg-target.h | 51 +-
tcg/ppc/tcg-target.opc.h | 13 +
tcg/ppc/tcg-target.inc.c | 1118 +++---
3 files changed, 1101 insertions(+), 81 deletions(-)
create mode 100644 tcg/ppc/tcg-target.opc.h
--
2.17.1
[PATCH v7 06/22] tcg/ppc: Replace HAVE_ISEL macro with a variable
Previously we've been hard-coding knowledge that Power7 has ISEL, but
it was an optional instruction before that. Use the AT_HWCAP2 bit,
when present, to properly determine support.
Signed-off-by: Richard Henderson
---
tcg/ppc/tcg-target.inc.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 7cb0002c14..db28ae7eb1 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -65,8 +65,7 @@
static tcg_insn_unit *tb_ret_addr;
TCGPowerISA have_isa;
-
-#define HAVE_ISEL have_isa_2_06
+static bool have_isel;
#ifndef CONFIG_SOFTMMU
#define TCG_GUEST_BASE_REG 30
@@ -1100,7 +1099,7 @@ static void tcg_out_setcond(TCGContext *s, TCGType type,
TCGCond cond,
/* If we have ISEL, we can implement everything with 3 or 4 insns.
All other cases below are also at least 3 insns, so speed up the
code generator by not considering them and always using ISEL. */
-if (HAVE_ISEL) {
+if (have_isel) {
int isel, tab;
tcg_out_cmp(s, cond, arg1, arg2, const_arg2, 7, type);
@@ -1203,7 +1202,7 @@ static void tcg_out_movcond(TCGContext *s, TCGType type,
TCGCond cond,
tcg_out_cmp(s, cond, c1, c2, const_c2, 7, type);
-if (HAVE_ISEL) {
+if (have_isel) {
int isel = tcg_to_isel[cond];
/* Swap the V operands if the operation indicates inversion. */
@@ -1247,7 +1246,7 @@ static void tcg_out_cntxz(TCGContext *s, TCGType type,
uint32_t opc,
} else {
tcg_out_cmp(s, TCG_COND_EQ, a1, 0, 1, 7, type);
/* Note that the only other valid constant for a2 is 0. */
-if (HAVE_ISEL) {
+if (have_isel) {
tcg_out32(s, opc | RA(TCG_REG_R0) | RS(a1));
tcg_out32(s, tcg_to_isel[TCG_COND_EQ] | TAB(a0, a2, TCG_REG_R0));
} else if (!const_a2 && a0 == a2) {
@@ -2795,6 +2794,14 @@ static void tcg_target_init(TCGContext *s)
}
#endif
+#ifdef PPC_FEATURE2_HAS_ISEL
+/* Prefer explicit instruction from the kernel. */
+have_isel = (hwcap2 & PPC_FEATURE2_HAS_ISEL) != 0;
+#else
+/* Fall back to knowing Power7 (2.06) has ISEL. */
+have_isel = have_isa_2_06;
+#endif
+
tcg_target_available_regs[TCG_TYPE_I32] = 0x;
tcg_target_available_regs[TCG_TYPE_I64] = 0x;
--
2.17.1
[PATCH v7 01/22] tcg/ppc: Introduce Altivec registers
Altivec supports 32 128-bit vector registers, whose names are
by convention v0 through v31.
Signed-off-by: Richard Henderson
Signed-off-by: Aleksandar Markovic
---
tcg/ppc/tcg-target.h | 11 -
tcg/ppc/tcg-target.inc.c | 88 +---
2 files changed, 65 insertions(+), 34 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 7627fb62d3..690fa744e1 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -31,7 +31,7 @@
# define TCG_TARGET_REG_BITS 32
#endif
-#define TCG_TARGET_NB_REGS 32
+#define TCG_TARGET_NB_REGS 64
#define TCG_TARGET_INSN_UNIT_SIZE 4
#define TCG_TARGET_TLB_DISPLACEMENT_BITS 16
@@ -45,6 +45,15 @@ typedef enum {
TCG_REG_R24, TCG_REG_R25, TCG_REG_R26, TCG_REG_R27,
TCG_REG_R28, TCG_REG_R29, TCG_REG_R30, TCG_REG_R31,
+TCG_REG_V0, TCG_REG_V1, TCG_REG_V2, TCG_REG_V3,
+TCG_REG_V4, TCG_REG_V5, TCG_REG_V6, TCG_REG_V7,
+TCG_REG_V8, TCG_REG_V9, TCG_REG_V10, TCG_REG_V11,
+TCG_REG_V12, TCG_REG_V13, TCG_REG_V14, TCG_REG_V15,
+TCG_REG_V16, TCG_REG_V17, TCG_REG_V18, TCG_REG_V19,
+TCG_REG_V20, TCG_REG_V21, TCG_REG_V22, TCG_REG_V23,
+TCG_REG_V24, TCG_REG_V25, TCG_REG_V26, TCG_REG_V27,
+TCG_REG_V28, TCG_REG_V29, TCG_REG_V30, TCG_REG_V31,
+
TCG_REG_CALL_STACK = TCG_REG_R1,
TCG_AREG0 = TCG_REG_R27
} TCGReg;
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 815edac077..9d678c3bf1 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -42,6 +42,9 @@
# define TCG_REG_TMP1 TCG_REG_R12
#endif
+#define TCG_VEC_TMP1TCG_REG_V0
+#define TCG_VEC_TMP2TCG_REG_V1
+
#define TCG_REG_TB TCG_REG_R31
#define USE_REG_TB (TCG_TARGET_REG_BITS == 64)
@@ -72,39 +75,15 @@ bool have_isa_3_00;
#endif
#ifdef CONFIG_DEBUG_TCG
-static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
-"r0",
-"r1",
-"r2",
-"r3",
-"r4",
-"r5",
-"r6",
-"r7",
-"r8",
-"r9",
-"r10",
-"r11",
-"r12",
-"r13",
-"r14",
-"r15",
-"r16",
-"r17",
-"r18",
-"r19",
-"r20",
-"r21",
-"r22",
-"r23",
-"r24",
-"r25",
-"r26",
-"r27",
-"r28",
-"r29",
-"r30",
-"r31"
+static const char tcg_target_reg_names[TCG_TARGET_NB_REGS][4] = {
+"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+"v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7",
+"v8", "v9", "v10", "v11", "v12", "v13", "v14", "v15",
+"v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
+"v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31",
};
#endif
@@ -139,6 +118,26 @@ static const int tcg_target_reg_alloc_order[] = {
TCG_REG_R5,
TCG_REG_R4,
TCG_REG_R3,
+
+/* V0 and V1 reserved as temporaries; V20 - V31 are call-saved */
+TCG_REG_V2, /* call clobbered, vectors */
+TCG_REG_V3,
+TCG_REG_V4,
+TCG_REG_V5,
+TCG_REG_V6,
+TCG_REG_V7,
+TCG_REG_V8,
+TCG_REG_V9,
+TCG_REG_V10,
+TCG_REG_V11,
+TCG_REG_V12,
+TCG_REG_V13,
+TCG_REG_V14,
+TCG_REG_V15,
+TCG_REG_V16,
+TCG_REG_V17,
+TCG_REG_V18,
+TCG_REG_V19,
};
static const int tcg_target_call_iarg_regs[] = {
@@ -2808,6 +2807,27 @@ static void tcg_target_init(TCGContext *s)
tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_R11);
tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_R12);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V0);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V1);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V2);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V3);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V4);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V5);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V6);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V7);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V8);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V9);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V10);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V11);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V12);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V13);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V14);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V15);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V16);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V17);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V18);
+tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_V19);
RE: [PATCH v5 3/4] net/filter.c: Add Options to insert filters anywhere in the filter list
> -Original Message-
> From: Lukas Straub
> Sent: Saturday, September 28, 2019 6:45 PM
> To: Zhang, Chen
> Cc: qemu-devel ; Jason Wang
> ; Wen Congyang ;
> Xie Changlong ; kw...@redhat.com;
> mre...@redhat.com
> Subject: Re: [PATCH v5 3/4] net/filter.c: Add Options to insert filters
> anywhere in the filter list
>
> On Thu, 26 Sep 2019 17:02:58 +
> "Zhang, Chen" wrote:
> > > diff --git a/qemu-options.hx b/qemu-options.hx index
> > > 08749a3391..23fa5a344e 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -4368,7 +4368,7 @@ applications, they can do this through this
> > > parameter. Its format is a gnutls priority string as described at
> > > @url{https://gnutls.org/manual/html_node/Priority-Strings.html}.
> > >
> > > -@item -object filter-
> > >
> buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@va
> > > r{
> > > all|rx|tx}][,status=@var{on|off}]
> > > +@item -object
> > > +filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,q
> > > +ueue
> > > +=@var{all|rx|tx}][,status=@var{on|off}][,position=@var{head|tail|id
> > > += > > +>}][,insert=@var{behind|before}]
> > >
> > > Interval @var{t} can't be 0, this filter batches the packet
> > > delivery: all packets arriving in a given interval on netdev
> > > @var{netdevid} are delayed @@ -
> > > 4387,11 +4387,11 @@ queue @var{all|rx|tx} is an option that can be
> > > applied to any netfilter.
> > > @option{tx}: the filter is attached to the transmit queue of the netdev,
> > > where it will receive packets sent by the netdev.
> > >
> > > -@item -object filter-
> > >
> mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queu
> > > e
> > > =@var{all|rx|tx}[,vnet_hdr_support]
> > > +@item -object
> > > +filter-
> mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardev
> > > +id},
> > > +queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|i
> > > +d=<
> > > i
> > > +d>}][,insert=@var{behind|before}]
> > >
> > > filter-mirror on netdev @var{netdevid},mirror net packet to
> > > chardev@var{chardevid}, if it has the vnet_hdr_support flag,
> > > filter-mirror will mirror packet with vnet_hdr_len.
> > >
> >
> > Please add description for the newly added parameter in each filter.
> > After that:
> > Reviewed-by: Zhang Chen
> >
> > Thanks
> > Zhang Chen
>
> Hi,
> I will add a single description like its currently done with the "queue"
> option,
> noting that it applies to any netfilter. Is that Ok?
It is enough for me.
Thanks
Zhang Chen
>
> Regards,
> Lukas Straub
>
> >
> > > -@item -object filter-
> > >
> redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},o
> > > ut dev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
> > > +@item -object
> > > +filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{char
> > > +devi
> > >
> +d},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][
> > > +,p
> > > os
> > > +ition=@var{head|tail|id=}][,insert=@var{behind|before}]
> > >
> > > filter-redirector on netdev @var{netdevid},redirect filter's net
> > > packet to chardev @var{chardevid},and redirect indev's packet to
> > > filter.if it has the vnet_hdr_support flag, @@ -4400,7 +4400,7 @@
> > > Create a filter-redirector we need to differ outdev id from indev
> > > id, id can not be the same. we can just use indev or outdev, but at
> > > least one of indev or outdev need to be specified.
> > >
> > > -@item -object filter-
> > > rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vn
> > > et_
> > > hdr_support]
> > > +@item -object
> > > +filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx
> > > +|tx}
> > > +,[vnet_hdr_support][,position=@var{head|tail|id=}][,insert=@var
> > > +{beh
> > > +ind|before}]
> > >
> > > Filter-rewriter is a part of COLO project.It will rewrite tcp
> > > packet to secondary from primary to keep secondary tcp
> > > connection,and rewrite @@ -
> > > 4413,7 +4413,7 @@ colo secondary:
> > > -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
> > > -object filter-rewriter,id=rew0,netdev=hn0,queue=all
> > >
> > > -@item -object filter-
> > >
> dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var
> > > {
> > > len}]
> > > +@item -object
> > > +filter-
> > > dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=
> > > +@var{len}][,position=@var{head|tail|id=}][,insert=@var{behind|b
> > > +efor
> > > +e}]
> > >
> > > Dump the network traffic on netdev @var{dev} to the file specified
> > > by @var{filename}. At most @var{len} bytes (64k by default) per
> > > packet are stored.
> > > --
> > > 2.20.1
> >
RE: [PATCH v5 2/4] tests/test-replication.c: Add test for ignoring requests after failover
> -Original Message-
> From: Lukas Straub
> Sent: Saturday, September 28, 2019 7:07 PM
> To: Zhang, Chen
> Cc: qemu-devel ; Jason Wang
> ; Wen Congyang ;
> Xie Changlong ; kw...@redhat.com;
> mre...@redhat.com
> Subject: Re: [PATCH v5 2/4] tests/test-replication.c: Add test for ignoring
> requests after failover
>
> On Thu, 26 Sep 2019 17:40:03 +
> "Zhang, Chen" wrote:
>
> > > -Original Message-
> > > From: Lukas Straub
> > > Sent: Monday, September 16, 2019 3:20 AM
> > > To: qemu-devel
> > > Cc: Zhang, Chen ; Jason Wang
> > > ; Wen Congyang
> ; Xie
> > > Changlong ; kw...@redhat.com;
> > > mre...@redhat.com
> > > Subject: [PATCH v5 2/4] tests/test-replication.c: Add test for
> > > ignoring requests after failover
> > >
> > > This simulates the case that happens when we resume COLO after
> failover.
> > >
> >
> > It looks change the title to "Add test for secondary node continuous
> backup" is better.
>
> Did you mean "continuous replication"? Would "Add test for secondary node
> continuing replication" be Ok?
OK for me.
Thanks
Zhang Chen
>
> > > Signed-off-by: Lukas Straub
> > > ---
> > > tests/test-replication.c | 52
> > >
> > > 1 file changed, 52 insertions(+)
> > >
> > > diff --git a/tests/test-replication.c b/tests/test-replication.c
> > > index
> > > f085d1993a..5addfc2227 100644
> > > --- a/tests/test-replication.c
> > > +++ b/tests/test-replication.c
> > > @@ -489,6 +489,56 @@ static void test_secondary_stop(void)
> > > teardown_secondary();
> > > }
> > >
> > > +static void test_secondary_failover_then_ignore_requests(void)
> >
> > Same as title, I think change to "test_secondary_continuous_backup" is
> more clear.
> >
> > Thanks
> > Zhang Chen
> >
> > > +{
> > > +BlockBackend *top_blk, *local_blk;
> > > +Error *local_err = NULL;
> > > +
> > > +top_blk = start_secondary();
> > > +replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
> > > +g_assert(!local_err);
> > > +
> > > +/* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> > > +local_blk = blk_by_name(S_LOCAL_DISK_ID);
> > > +test_blk_write(local_blk, 0x22, IMG_SIZE / 2, IMG_SIZE / 2,
> > > + false);
> > > +
> > > +/* replication will backup s_local_disk to s_hidden_disk */
> > > +test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
> > > + IMG_SIZE / 2, 0, IMG_SIZE, false);
> > > +
> > > +/* write 0x33 to s_active_disk (0, IMG_SIZE / 2) */
> > > +test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
> > > +
> > > +/* do failover (active commit) */
> > > +replication_stop_all(true, &local_err);
> > > +g_assert(!local_err);
> > > +
> > > +/* it should ignore all requests from now on */
> > > +
> > > +/* start after failover */
> > > +replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
> > > +g_assert(!local_err);
> > > +
> > > +/* checkpoint */
> > > +replication_do_checkpoint_all(&local_err);
> > > +g_assert(!local_err);
> > > +
> > > +/* stop */
> > > +replication_stop_all(true, &local_err);
> > > +g_assert(!local_err);
> > > +
> > > +/* read from s_local_disk (0, IMG_SIZE / 2) */
> > > +test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
> > > + 0, IMG_SIZE / 2, false);
> > > +
> > > +
> > > +/* read from s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> > > +test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
> > > + IMG_SIZE / 2, 0, IMG_SIZE, false);
> > > +
> > > +teardown_secondary();
> > > +}
> > > +
> > > static void test_secondary_do_checkpoint(void)
> > > {
> > > BlockBackend *top_blk, *local_blk; @@ -584,6 +634,8 @@ int
> > > main(int argc, char **argv)
> > > g_test_add_func("/replication/secondary/write",
> test_secondary_write);
> > > g_test_add_func("/replication/secondary/start",
> test_secondary_start);
> > > g_test_add_func("/replication/secondary/stop",
> > > test_secondary_stop);
> > > +
> > > g_test_add_func("/replication/secondary/failover_then_ignore_request
> > > s",
> > > +test_secondary_failover_then_ignore_requests);
> > > g_test_add_func("/replication/secondary/do_checkpoint",
> > > test_secondary_do_checkpoint);
> > > g_test_add_func("/replication/secondary/get_error_all",
> > > --
> > > 2.20.1
> >
RE: [PATCH v5 4/4] colo: Update Documentation for continuous replication
> -Original Message- > From: Lukas Straub > Sent: Saturday, September 28, 2019 9:25 PM > To: Zhang, Chen > Cc: qemu-devel ; Jason Wang > ; Wen Congyang ; > Xie Changlong ; kw...@redhat.com; > mre...@redhat.com > Subject: Re: [PATCH v5 4/4] colo: Update Documentation for continuous > replication > > On Thu, 26 Sep 2019 17:27:35 + > "Zhang, Chen" wrote: > > > > -Original Message- > > > From: Lukas Straub > > > Sent: Monday, September 16, 2019 3:20 AM > > > To: qemu-devel > > > Cc: Zhang, Chen ; Jason Wang > > > ; Wen Congyang > ; Xie > > > Changlong ; kw...@redhat.com; > > > mre...@redhat.com > > > Subject: [PATCH v5 4/4] colo: Update Documentation for continuous > > > replication > > > > > > Document the qemu command-line and qmp commands for continuous > > > replication > > > > > > Signed-off-by: Lukas Straub > > > --- > > > docs/COLO-FT.txt | 212 +++-- > > > docs/block-replication.txt | 28 +++-- > > > 2 files changed, 173 insertions(+), 67 deletions(-) > > > > > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index > > > ad24680d13..e98c95ca2b 100644 > > > --- a/docs/COLO-FT.txt > > > +++ b/docs/COLO-FT.txt > > > @@ -145,35 +145,65 @@ The diagram just shows the main qmp > command, > > > you can get the detail in test procedure. > > > > > > == Test procedure == > > > -1. Startup qemu > > > -Primary: > > > -# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name > > > primary \ > > > - -device piix3-usb-uhci -vnc :7 \ > > > - -device usb-tablet -netdev tap,id=hn0,vhost=off \ > > > - -device virtio-net-pci,id=net-pci0,netdev=hn0 \ > > > - -drive > > > if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote- > > > threshold=1,\ > > > - children.0.file.filename=1.raw,\ > > > - children.0.driver=raw -S > > > -Secondary: > > > -# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name > > > secondary \ > > > - -device piix3-usb-uhci -vnc :7 \ > > > - -device usb-tablet -netdev tap,id=hn0,vhost=off \ > > > - -device virtio-net-pci,id=net-pci0,netdev=hn0 \ > > > - -drive > > > if=none,id=secondary-disk0,file.filename=1.raw,driver=raw,node- > > > name=node0 \ > > > - -drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\ > > > - file.driver=qcow2,top-id=active-disk0,\ > > > - file.file.filename=/mnt/ramfs/active_disk.img,\ > > > - file.backing.driver=qcow2,\ > > > - file.backing.file.filename=/mnt/ramfs/hidden_disk.img,\ > > > - file.backing.backing=secondary-disk0 \ > > > - -incoming tcp:0: > > > - > > > -2. On Secondary VM's QEMU monitor, issue command > > > +Note: Here we are running both instances on the same Machine for > > > +testing, change the IP Addresses if you want to run it on two Hosts > > > + > > > +== Startup qemu == > > > +1. Primary: > > > +# imagefolder="/mnt/vms/colo-test" > > > + > > > +The disks for the primary and secondary need to be the same before > > > +starting colo # cp --reflink=auto $imagefolder/primary.qcow2 > > > +$imagefolder/primary-copy.qcow2 > > > + > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 - > smp > > > 1 -qmp stdio \ > > > + -vnc :0 -device piix3-usb-uhci -device usb-tablet -name primary \ > > > + -netdev > > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper > > > \ > > > + -device rtl8139,id=e0,netdev=hn0 \ > > > + -chardev socket,id=mirror0,host=127.0.0.1,port=9003,server,nowait \ > > > + -chardev socket,id=compare1,host=127.0.0.1,port=9004,server,wait \ > > > + -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait > \ > > > + -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \ > > > + -chardev > > > + socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait > > > \ > > > + -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \ > > > + -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \ > > > + -object filter- > > > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \ > > > + -object filter- > > > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \ > > > + -object iothread,id=iothread1 \ > > > + -object > > > +colo-compare,id=comp0,primary_in=compare0- > > > 0,secondary_in=compare1,\ > > > +outdev=compare_out0,iothread=iothread1 \ > > > + -drive > > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold > > > +=1,\ > > > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driv > > > +er=q > > > +cow2 -S > > > + > > > +2. Secondary: > > > +# imagefolder="/mnt/vms/colo-test" > > > + > > > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G > > > + > > > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2 > 10G > > > + > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 - > smp > > > 1 -qmp stdio \ > > > + -vnc :1 -device piix3-usb-uhci -device usb-tablet -name secondary \ > > > + -netdev > > >
Re: Thoughts on VM fence infrastructure
>>> There are times when the main loop can get blocked even though the CPU >>> threads can be running and can in some configurations perform IO >>> even without the main loop (I think!). >> Ah, that's a very good point. Indeed, you can perform IO in those >> cases specially when using vhost devices. >> >>> By setting a timer in the kernel that sends a signal to qemu, the kernel >>> will send that signal however broken qemu is. >> Got you now. That's probably better. Do you reckon a signal is >> preferable over SIGEV_THREAD? > Not sure; probably the safest is getting the kernel to SIGKILL it - but > that's a complete nightmare to debug - your process just goes *pop* > with no apparent reason why. > I've not used SIGEV_THREAD - it looks promising though. Sorry to "enter" the discussion, but, in "real" HW, its not by accident that watchdog devices timeout generates a NMI to CPUs, causing the kernel to handle the interrupt - and panic (or to take other action set by specific watchdog drivers that re-implements the default ones). Can't you simple "inject" a NMI in all guest vCPUs BEFORE you take any action in QEMU itself? Just like the virtual watchdog device would do, from inside the guest (/dev/watchdog), but capable of being updated by outside, in this case of yours (if I understood correctly). Possibly you would have to have a dedicated loop for this "watchdog device" (AIO threads ?) not to compete with existing coroutines/BH Tasks and their jittering on your "realtime watchdog needs". Regarding remaining existing I/OS for the guest's devices in question (vhost/vhost-user etc), would be just like a real host where the "bus" received commands, but sender died right after...
Re: [PATCH] user-exec: Do not filter the signal on si_code
Patchew URL: https://patchew.org/QEMU/20190930192931.20509-1-richard.hender...@linaro.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190930192931.20509-1-richard.hender...@linaro.org/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH] user-exec: Do not filter the signal on si_code
This is a workaround for a ppc64le host kernel bug.
For the test case linux-test, we have an instruction trace
IN: sig_alarm
...
IN:
0x400080ed28: 38ac li r0, 0xac
0x400080ed2c: 4402 sc
IN: __libc_nanosleep
0x1003bb4c: 7c0802a6 mflr r0
0x1003bb50: f8010010 std r0, 0x10(r1)
Our signal return trampoline has, rightly, changed the guest
stack page read-only. Which, rightly, faults on the store of
a return address into a stack frame.
Checking the host /proc/pid/maps, we see the expected state:
400080-400081 r--p 00:00 0
However, the host kernel has supplied si_code == SEGV_MAPERR,
which is obviously incorrect.
By dropping this check, we may have an extra walk of the page
tables, but this should be inexpensive.
Signed-off-by: Richard Henderson
---
FWIW, filed as
https://bugzilla.redhat.com/show_bug.cgi?id=1757189
out of habit and then
https://bugs.centos.org/view.php?id=16499
when I remembered that the system is running Centos not RHEL.
---
accel/tcg/user-exec.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 71c4bf6477..31ef091a70 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -143,9 +143,12 @@ static inline int handle_cpu_signal(uintptr_t pc,
siginfo_t *info,
* for some other kind of fault that should really be passed to the
* guest, we'd end up in an infinite loop of retrying the faulting
* access.
+ *
+ * XXX: At least one host kernel, ppc64le w/Centos 7 4.14.0-115.6.1,
+ * incorrectly reports SEGV_MAPERR for a STDX write to a read-only page.
+ * Therefore, do not test info->si_code.
*/
-if (is_write && info->si_signo == SIGSEGV && info->si_code == SEGV_ACCERR
&&
-h2g_valid(address)) {
+if (is_write && info->si_signo == SIGSEGV && h2g_valid(address)) {
switch (page_unprotect(h2g(address), pc)) {
case 0:
/* Fault not caused by a page marked unwritable to protect
--
2.17.1
Re: [PATCH v2 29/33] spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate
On 30/09/2019 10:25, David Gibson wrote: > On Mon, Sep 30, 2019 at 08:11:56AM +0200, Cédric Le Goater wrote: >> On 27/09/2019 07:50, David Gibson wrote: >>> It turns out that all the logic in the SpaprIrq::reset hooks (and some in >>> the SpaprIrq::post_load hooks) isn't really related to resetting the irq >>> backend (that's handled by the backends' own reset routines). Rather its >>> about getting the backend ready to be the active interrupt controller or >>> stopping being the active interrupt controller - reset (and post_load) is >>> just the only time that changes at present. >> >> This is a 'critical' part which impacts all the migration cases: >> >> ic-mode=xics,xive,dual + kernel_irqchip=on/off + TCG > > Yes... and? and it's fragile. >>> To make this flow clearer, move the logic into the explicit backend >>> activate and deactivate hooks. >> >> I don't see where the hooks are called ? > > spapr_irq_reset() > -> spapr_irq_update_active_intc() > -> set_active_intc() > -> activate/deactivate hooks > > Similarly via spapr_irq_post_load(). > > I'm hoping to add one at CAS time to avoid the CAS reboot, too. OK. I think the first 22 patches are ready, just some minor comments if I am correct. Could you merge them ? C.
Qemu Dirty Bitmap backup to encrypted target
How can have QEMU backup write the output to an encrypted target?
Blocks in the dirty bitmap are unencrypted, and as such when I write them with QEMU backup they are written to the target unencrypted.
I've experimented with providing a json string as the target but with no luck.
transaction='{ "execute": "transaction",
"arguments": {
"actions": [
{"type": "block-dirty-bitmap-add",
"data": {"node": "drive-virtio-disk0", "granularity": 2097152, "name": "mybitmap"} },
{"type": "drive-backup",
"data": {"device": "drive-virtio-disk0", "target": "json:{\"encrypt.format\": \"luks\", \"encrypt.key-secret\": \"virtio-disk0-luks-secret0\", \"driver\": \"qcow2\", \"file\": {\"driver\": \"file\", \"filename\": \"/tmp/target-encrypt-test.qcow2\"}}",
"sync": "full", "format": "qcow2"} }
]
}
}'
virsh -c qemu:///system qemu-monitor-command --pretty 28 $transaction
{
"id": "libvirt-45",
"error": {
"class": "GenericError",
"desc": "Unknown protocol 'json'"
}
}
___Craig MullVPC Storage Architectcm...@us.ibm.com
Re: Thoughts on VM fence infrastructure
> On Sep 30, 2019, at 6:59 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> >> >>> On Sep 30, 2019, at 6:11 PM, Dr. David Alan Gilbert >>> wrote: >>> >>> * Felipe Franciosi (fel...@nutanix.com) wrote: > On Sep 30, 2019, at 5:03 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> Hi David, >> >>> On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert >>> wrote: >>> >>> * Felipe Franciosi (fel...@nutanix.com) wrote: Heyall, We have a use case where a host should self-fence (and all VMs should die) if it doesn't hear back from a heartbeat within a certain time period. Lots of ideas were floated around where libvirt could take care of killing VMs or a separate service could do it. The concern with those is that various failures could lead to _those_ services being unavailable and the fencing wouldn't be enforced as it should. Ultimately, it feels like Qemu should be responsible for this heartbeat and exit (or execute a custom callback) on timeout. >>> >>> It doesn't feel doing it inside qemu would be any safer; something >>> outside QEMU can forcibly emit a kill -9 and qemu *will* stop. >> >> The argument above is that we would have to rely on this external >> service being functional. Consider the case where the host is >> dysfunctional, with this service perhaps crashed and a corrupt >> filesystem preventing it from restarting. The VMs would never die. > > Yeh that could fail. > >> It feels like a Qemu timer-driven heartbeat check and calls abort() / >> exit() would be more reliable. Thoughts? > > OK, yes; perhaps using a timer_create and telling it to send a fatal > signal is pretty solid; it would take the kernel to do that once it's > set. I'm confused about why the kernel needs to be involved. If this is a timer off the Qemu main loop, it can just check on the heartbeat condition (which should be customisable) and call abort() if that's not satisfied. If you agree on that I'd like to talk about how that check could be made customisable. >>> >>> There are times when the main loop can get blocked even though the CPU >>> threads can be running and can in some configurations perform IO >>> even without the main loop (I think!). >> >> Ah, that's a very good point. Indeed, you can perform IO in those >> cases specially when using vhost devices. >> >>> By setting a timer in the kernel that sends a signal to qemu, the kernel >>> will send that signal however broken qemu is. >> >> Got you now. That's probably better. Do you reckon a signal is >> preferable over SIGEV_THREAD? > > Not sure; probably the safest is getting the kernel to SIGKILL it - but > that's a complete nightmare to debug - your process just goes *pop* > with no apparent reason why. > I've not used SIGEV_THREAD - it looks promising though. I'm worried that SIGEV_THREAD could be a bit heavyweight (if it fires up a new thread each time). On the other hand, as you said, SIGKILL makes it harder to debug. Also, asking the kernel to defer the SIGKILL (ie. updating the timer) needs to come from Qemu itself (eg. a timer in the main loop, something we already ruled unsuitable, or a qmp command which constitutes an external dependency that we also ruled undesirable). What if, when self-fencing is enabled, Qemu kicks off a new thread from the start which does nothing but periodically wake up, verify the heartbeat condition and log()+abort() if required? (Then we wouldn't need the kernel timer.) > >> I'm still wondering how to make this customisable so that different >> types of heartbeat could be implemented (preferably without creating >> external dependencies per discussion above). Thoughts welcome. > > Yes, you need something to enable it, and some safe way to retrigger > the timer. A qmp command marked as 'oob' might be the right way - > another qm command can't block it. This qmp approach is slightly different than the external dependency that itself kills Qemu; if it doesn't run, then Qemu dies because the kernel timer is not updated. But this is also a heavyweight approach. We are talking about a service that needs to frequently connect to all running VMs on a host to reset the timer. But it does allow for the customisable heartbeat: the logic behind what triggers the command is completely flexible. Thinking about this idea of a separate Qemu thread, one thing that came to mind is this: qemu -fence heartbeat=/path/to/file,deadline=60[,recheck=5] Qemu could fire up a thread that stat()s (every seconds or on a default interval) and log()+abort() the whole process if the last modification time of the file is older than . If goes away (ie. stat() gives ENOENT), then it either fences immedia
Re: [PATCH] x86: Add CPUID KVM support for new instruction WBNOINVD
On Mon, Sep 30, 2019 at 10:54 AM Eduardo Habkost wrote: > > CCing qemu-devel. > > On Tue, Sep 24, 2019 at 01:30:04PM -0700, Jim Mattson wrote: > > On Wed, Dec 19, 2018 at 1:02 PM Paolo Bonzini wrote: > > > > > > On 19/12/18 18:39, Jim Mattson wrote: > > > > Is this an instruction that kvm has to be able to emulate before it > > > > can enumerate its existence? > > > > > > It doesn't have any operands, so no. > > > > > > Paolo > > > > > > > On Wed, Dec 19, 2018 at 5:51 AM Robert Hoo > > > > wrote: > > > >> > > > >> Signed-off-by: Robert Hoo > > > >> --- > > > >> arch/x86/include/asm/cpufeatures.h | 1 + > > > >> arch/x86/kvm/cpuid.c | 2 +- > > > >> 2 files changed, 2 insertions(+), 1 deletion(-) > > > >> > > > >> diff --git a/arch/x86/include/asm/cpufeatures.h > > > >> b/arch/x86/include/asm/cpufeatures.h > > > >> index 28c4a50..932b19f 100644 > > > >> --- a/arch/x86/include/asm/cpufeatures.h > > > >> +++ b/arch/x86/include/asm/cpufeatures.h > > > >> @@ -280,6 +280,7 @@ > > > >> /* AMD-defined CPU features, CPUID level 0x8008 (EBX), word 13 */ > > > >> #define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO > > > >> instruction */ > > > >> #define X86_FEATURE_IRPERF (13*32+ 1) /* Instructions > > > >> Retired Count */ > > > >> +#define X86_FEATURE_WBNOINVD (13*32+ 9) /* Writeback and > > > >> Don't invalid cache */ > > > >> #define X86_FEATURE_XSAVEERPTR (13*32+ 2) /* Always > > > >> save/restore FP error pointers */ > > > >> #define X86_FEATURE_AMD_IBPB (13*32+12) /* "" Indirect > > > >> Branch Prediction Barrier */ > > > >> #define X86_FEATURE_AMD_IBRS (13*32+14) /* "" Indirect > > > >> Branch Restricted Speculation */ > > > >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > > >> index cc6dd65..763e115 100644 > > > >> --- a/arch/x86/kvm/cpuid.c > > > >> +++ b/arch/x86/kvm/cpuid.c > > > >> @@ -380,7 +380,7 @@ static inline int __do_cpuid_ent(struct > > > >> kvm_cpuid_entry2 *entry, u32 function, > > > >> > > > >> /* cpuid 0x8008.ebx */ > > > >> const u32 kvm_cpuid_8000_0008_ebx_x86_features = > > > >> - F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) > > > >> | > > > >> + F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) > > > >> | F(VIRT_SSBD) | > > > >> F(AMD_SSB_NO) | F(AMD_STIBP); > > > >> > > > >> /* cpuid 0xC001.edx */ > > > >> -- > > > >> 1.8.3.1 > > > >> > > > > What is the point of enumerating support for WBNOINVD if kvm is going > > to implement it as WBINVD? > > I expect GET_SUPPORTED_CPUID to return WBNOINVD, because it > indicates to userspace what is supported by KVM. Are there any > expectations that GET_SUPPORTED_CPUID will also dictate what is > enabled by default in some cases? > > In either case, your question applies to QEMU: why do we want > WBNOINVD to be enabled by "-cpu host" by default and be part of > QEMU's Icelake-* CPU model definitions? I had only looked at the SVM implementation of WBNOINVD, which is exactly the same as the SVM implementation of WBINVD. So, the question is, "why enumerate WBNOINVD if its implementation is exactly the same as WBINVD?" WBNOINVD appears to be only partially documented in Intel document 319433-037, "Intel® Architecture Instruction Set Extensions and Future Features Programming Reference." In particular, there is no documentation regarding the instruction's behavior in VMX non-root mode. Does WBNOINVD cause a VM-exit when the VM-execution control, "WBINVD exiting," is set? If so, does it have the same VM-exit reason as WBINVD (54), or a different one? If it does have the same VM-exit reason (a la SVM), how does one distinguish a WBINVD VM-exit from a WBNOINVD VM-exit? If one can't distinguish (a la SVM), then it would seem that the VMX implementation also implements WBNOINVD as WBINVD. If that's the case, the question for VMX is the same as for SVM.
Re: Lockup with --accel tcg,thread=single
Paolo Bonzini writes:
> On 30/09/19 17:37, Peter Maydell wrote:
>> Not sure currently what the best fix is here.
>
> Since thread=single uses just one queued work list, could it be as
> simple as
Does it? I thought this was the case but:
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
{
qemu_mutex_lock(&cpu->work_mutex);
if (cpu->queued_work_first == NULL) {
cpu->queued_work_first = wi;
} else {
cpu->queued_work_last->next = wi;
}
cpu->queued_work_last = wi;
wi->next = NULL;
wi->done = false;
qemu_mutex_unlock(&cpu->work_mutex);
qemu_cpu_kick(cpu);
}
Does seem to imply the vCPU CPUState is where the queue is. That's not
to say there shouldn't be a single work queue for thread=single.
>
> diff --git a/cpus.c b/cpus.c
> index d2c61ff..314f9aa 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1539,7 +1539,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
> cpu = first_cpu;
> }
>
> -while (cpu && !cpu->queued_work_first && !cpu->exit_request) {
> +while (cpu && !first_cpu->queued_work_first && !cpu->exit_request) {
>
> atomic_mb_set(&tcg_current_rr_cpu, cpu);
> current_cpu = cpu;
>
> or something like that?
>
>> Side note -- this use of run_on_cpu() means that we now drop
>> the iothread lock within memory_region_snapshot_and_clear_dirty(),
>> which we didn't before. This means that a vCPU thread can now
>> get in and execute an access to the device registers of
>> hw/display/vga.c, updating its state in a way I suspect that the
>> device model code is not expecting... So maybe the right answer
>> here should be to come up with a fix for the race that 9458a9a1
>> addresses that doesn't require us to drop the iothread lock in
>> memory_region_snapshot_and_clear_dirty() ? Alternatively we need
>> to audit the callers and flag in the documentation that this
>> function has the unexpected side effect of briefly dropping the
>> iothread lock.
>
> Yes, this is intended. There shouldn't be side effects other than
> possibly a one-frame flash for all callers.
>
> Paolo
--
Alex Bennée
Re: [PATCH v2 4/7] riscv/sifive_u: Add the start-in-flash property
On Fri, Sep 27, 2019 at 12:57 AM Bin Meng wrote:
>
> On Fri, Sep 27, 2019 at 8:55 AM Alistair Francis
> wrote:
> >
> > Add a property that when set to true QEMU will jump from the ROM code to
> > the start of flash memory instead of DRAM which is the default
> > behaviour.
> >
> > Signed-off-by: Alistair Francis
> > ---
> > hw/riscv/sifive_u.c | 27 +++
> > include/hw/riscv/sifive_u.h | 2 ++
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index f5741e9a38..33b55d0d5b 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -373,6 +373,10 @@ static void riscv_sifive_u_init(MachineState *machine)
> > /* dtb: */
> > };
> >
> > +if (s->start_in_flash) {
> > +reset_vec[6] = memmap[SIFIVE_U_FLASH0].base; /* start: .dword
> > FLASH0_BASE */
> > +}
>
> Please change to use the way that patch "[v2,7/7] riscv/virt: Jump to
> pflash if specified" does for consistency, ie:
>
> if (s->start_in_flash) {
> start_addr = memmap[SIFIVE_U_FLASH0].base; /* start: .dword
> FLASH0_BASE */
> }
>
> > +
> > /* copy in the reset vector in little_endian byte order */
> > for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > @@ -432,8 +436,31 @@ static void riscv_sifive_u_soc_init(Object *obj)
> >TYPE_CADENCE_GEM);
> > }
> >
> > +static bool virt_get_start_in_flash(Object *obj, Error **errp)
>
> sifive_u_get_start_in_flash()
>
> > +{
> > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +return s->start_in_flash;
> > +}
> > +
> > +static void virt_set_start_in_flash(Object *obj, bool value, Error **errp)
>
> sifive_u_set_start_in_flash()
Yep, I have fixed all of these.
Alistair
>
> > +{
> > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +s->start_in_flash = value;
> > +}
> > +
> > static void riscv_sifive_u_machine_instance_init(Object *obj)
> > {
> > +SiFiveUState *s = RISCV_U_MACHINE(obj);
> > +
> > +s->start_in_flash = false;
> > +object_property_add_bool(obj, "start-in-flash",
> > virt_get_start_in_flash,
> > + virt_set_start_in_flash, NULL);
> > +object_property_set_description(obj, "start-in-flash",
> > +"Set on to tell QEMU's ROM to jump to
> > " \
> > +"flash. Otherwise QEMU will jump to
> > DRAM",
> > +NULL);
> > }
> >
> > static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
> > diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
> > index a921079fbe..2656b43c58 100644
> > --- a/include/hw/riscv/sifive_u.h
> > +++ b/include/hw/riscv/sifive_u.h
> > @@ -57,6 +57,8 @@ typedef struct SiFiveUState {
> >
> > void *fdt;
> > int fdt_size;
> > +
> > +bool start_in_flash;
> > } SiFiveUState;
> >
>
> Regards,
> Bin
Re: Thoughts on VM fence infrastructure
* Felipe Franciosi (fel...@nutanix.com) wrote: > > > > On Sep 30, 2019, at 6:11 PM, Dr. David Alan Gilbert > > wrote: > > > > * Felipe Franciosi (fel...@nutanix.com) wrote: > >> > >> > >>> On Sep 30, 2019, at 5:03 PM, Dr. David Alan Gilbert > >>> wrote: > >>> > >>> * Felipe Franciosi (fel...@nutanix.com) wrote: > Hi David, > > > On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert > > wrote: > > > > * Felipe Franciosi (fel...@nutanix.com) wrote: > >> Heyall, > >> > >> We have a use case where a host should self-fence (and all VMs should > >> die) if it doesn't hear back from a heartbeat within a certain time > >> period. Lots of ideas were floated around where libvirt could take > >> care of killing VMs or a separate service could do it. The concern > >> with those is that various failures could lead to _those_ services > >> being unavailable and the fencing wouldn't be enforced as it should. > >> > >> Ultimately, it feels like Qemu should be responsible for this > >> heartbeat and exit (or execute a custom callback) on timeout. > > > > It doesn't feel doing it inside qemu would be any safer; something > > outside QEMU can forcibly emit a kill -9 and qemu *will* stop. > > The argument above is that we would have to rely on this external > service being functional. Consider the case where the host is > dysfunctional, with this service perhaps crashed and a corrupt > filesystem preventing it from restarting. The VMs would never die. > >>> > >>> Yeh that could fail. > >>> > It feels like a Qemu timer-driven heartbeat check and calls abort() / > exit() would be more reliable. Thoughts? > >>> > >>> OK, yes; perhaps using a timer_create and telling it to send a fatal > >>> signal is pretty solid; it would take the kernel to do that once it's > >>> set. > >> > >> I'm confused about why the kernel needs to be involved. If this is a > >> timer off the Qemu main loop, it can just check on the heartbeat > >> condition (which should be customisable) and call abort() if that's > >> not satisfied. If you agree on that I'd like to talk about how that > >> check could be made customisable. > > > > There are times when the main loop can get blocked even though the CPU > > threads can be running and can in some configurations perform IO > > even without the main loop (I think!). > > Ah, that's a very good point. Indeed, you can perform IO in those > cases specially when using vhost devices. > > > By setting a timer in the kernel that sends a signal to qemu, the kernel > > will send that signal however broken qemu is. > > Got you now. That's probably better. Do you reckon a signal is > preferable over SIGEV_THREAD? Not sure; probably the safest is getting the kernel to SIGKILL it - but that's a complete nightmare to debug - your process just goes *pop* with no apparent reason why. I've not used SIGEV_THREAD - it looks promising though. > I'm still wondering how to make this customisable so that different > types of heartbeat could be implemented (preferably without creating > external dependencies per discussion above). Thoughts welcome. Yes, you need something to enable it, and some safe way to retrigger the timer. A qmp command marked as 'oob' might be the right way - another qm command can't block it. Dave > F. > > > > >> > >>> > >>> IMHO the safer way is to kick the host off the network by reprogramming > >>> switches; so even if the qemu is actually alive it can't get anywhere. > >>> > >>> Dave > >> > >> Naturally some off-host STONITH is preferable, but that's not always > >> available. A self-fencing mechanism right at the heart of the emulator > >> can do the job without external hardware dependencies. > > > > Dave > > > >> Cheers, > >> Felipe > >> > >>> > >>> > Felipe > > > > >> Does something already exist for this purpose which could be used? > >> Would a generic Qemu-fencing infrastructure be something of interest? > > Dave > > > > > >> Cheers, > >> F. > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > >>> -- > >>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] x86: Add CPUID KVM support for new instruction WBNOINVD
CCing qemu-devel. On Tue, Sep 24, 2019 at 01:30:04PM -0700, Jim Mattson wrote: > On Wed, Dec 19, 2018 at 1:02 PM Paolo Bonzini wrote: > > > > On 19/12/18 18:39, Jim Mattson wrote: > > > Is this an instruction that kvm has to be able to emulate before it > > > can enumerate its existence? > > > > It doesn't have any operands, so no. > > > > Paolo > > > > > On Wed, Dec 19, 2018 at 5:51 AM Robert Hoo > > > wrote: > > >> > > >> Signed-off-by: Robert Hoo > > >> --- > > >> arch/x86/include/asm/cpufeatures.h | 1 + > > >> arch/x86/kvm/cpuid.c | 2 +- > > >> 2 files changed, 2 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/arch/x86/include/asm/cpufeatures.h > > >> b/arch/x86/include/asm/cpufeatures.h > > >> index 28c4a50..932b19f 100644 > > >> --- a/arch/x86/include/asm/cpufeatures.h > > >> +++ b/arch/x86/include/asm/cpufeatures.h > > >> @@ -280,6 +280,7 @@ > > >> /* AMD-defined CPU features, CPUID level 0x8008 (EBX), word 13 */ > > >> #define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO instruction > > >> */ > > >> #define X86_FEATURE_IRPERF (13*32+ 1) /* Instructions > > >> Retired Count */ > > >> +#define X86_FEATURE_WBNOINVD (13*32+ 9) /* Writeback and > > >> Don't invalid cache */ > > >> #define X86_FEATURE_XSAVEERPTR (13*32+ 2) /* Always > > >> save/restore FP error pointers */ > > >> #define X86_FEATURE_AMD_IBPB (13*32+12) /* "" Indirect Branch > > >> Prediction Barrier */ > > >> #define X86_FEATURE_AMD_IBRS (13*32+14) /* "" Indirect Branch > > >> Restricted Speculation */ > > >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > >> index cc6dd65..763e115 100644 > > >> --- a/arch/x86/kvm/cpuid.c > > >> +++ b/arch/x86/kvm/cpuid.c > > >> @@ -380,7 +380,7 @@ static inline int __do_cpuid_ent(struct > > >> kvm_cpuid_entry2 *entry, u32 function, > > >> > > >> /* cpuid 0x8008.ebx */ > > >> const u32 kvm_cpuid_8000_0008_ebx_x86_features = > > >> - F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | > > >> + F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | > > >> F(VIRT_SSBD) | > > >> F(AMD_SSB_NO) | F(AMD_STIBP); > > >> > > >> /* cpuid 0xC001.edx */ > > >> -- > > >> 1.8.3.1 > > >> > > What is the point of enumerating support for WBNOINVD if kvm is going > to implement it as WBINVD? I expect GET_SUPPORTED_CPUID to return WBNOINVD, because it indicates to userspace what is supported by KVM. Are there any expectations that GET_SUPPORTED_CPUID will also dictate what is enabled by default in some cases? In either case, your question applies to QEMU: why do we want WBNOINVD to be enabled by "-cpu host" by default and be part of QEMU's Icelake-* CPU model definitions? -- Eduardo
Re: Lockup with --accel tcg,thread=single
On 30/09/19 17:37, Peter Maydell wrote:
> Not sure currently what the best fix is here.
Since thread=single uses just one queued work list, could it be as simple as
diff --git a/cpus.c b/cpus.c
index d2c61ff..314f9aa 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1539,7 +1539,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
cpu = first_cpu;
}
-while (cpu && !cpu->queued_work_first && !cpu->exit_request) {
+while (cpu && !first_cpu->queued_work_first && !cpu->exit_request) {
atomic_mb_set(&tcg_current_rr_cpu, cpu);
current_cpu = cpu;
or something like that?
> Side note -- this use of run_on_cpu() means that we now drop
> the iothread lock within memory_region_snapshot_and_clear_dirty(),
> which we didn't before. This means that a vCPU thread can now
> get in and execute an access to the device registers of
> hw/display/vga.c, updating its state in a way I suspect that the
> device model code is not expecting... So maybe the right answer
> here should be to come up with a fix for the race that 9458a9a1
> addresses that doesn't require us to drop the iothread lock in
> memory_region_snapshot_and_clear_dirty() ? Alternatively we need
> to audit the callers and flag in the documentation that this
> function has the unexpected side effect of briefly dropping the
> iothread lock.
Yes, this is intended. There shouldn't be side effects other than
possibly a one-frame flash for all callers.
Paolo
Re: Thoughts on VM fence infrastructure
> On Sep 30, 2019, at 6:11 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> >> >>> On Sep 30, 2019, at 5:03 PM, Dr. David Alan Gilbert >>> wrote: >>> >>> * Felipe Franciosi (fel...@nutanix.com) wrote: Hi David, > On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> Heyall, >> >> We have a use case where a host should self-fence (and all VMs should >> die) if it doesn't hear back from a heartbeat within a certain time >> period. Lots of ideas were floated around where libvirt could take >> care of killing VMs or a separate service could do it. The concern >> with those is that various failures could lead to _those_ services >> being unavailable and the fencing wouldn't be enforced as it should. >> >> Ultimately, it feels like Qemu should be responsible for this >> heartbeat and exit (or execute a custom callback) on timeout. > > It doesn't feel doing it inside qemu would be any safer; something > outside QEMU can forcibly emit a kill -9 and qemu *will* stop. The argument above is that we would have to rely on this external service being functional. Consider the case where the host is dysfunctional, with this service perhaps crashed and a corrupt filesystem preventing it from restarting. The VMs would never die. >>> >>> Yeh that could fail. >>> It feels like a Qemu timer-driven heartbeat check and calls abort() / exit() would be more reliable. Thoughts? >>> >>> OK, yes; perhaps using a timer_create and telling it to send a fatal >>> signal is pretty solid; it would take the kernel to do that once it's >>> set. >> >> I'm confused about why the kernel needs to be involved. If this is a >> timer off the Qemu main loop, it can just check on the heartbeat >> condition (which should be customisable) and call abort() if that's >> not satisfied. If you agree on that I'd like to talk about how that >> check could be made customisable. > > There are times when the main loop can get blocked even though the CPU > threads can be running and can in some configurations perform IO > even without the main loop (I think!). Ah, that's a very good point. Indeed, you can perform IO in those cases specially when using vhost devices. > By setting a timer in the kernel that sends a signal to qemu, the kernel > will send that signal however broken qemu is. Got you now. That's probably better. Do you reckon a signal is preferable over SIGEV_THREAD? I'm still wondering how to make this customisable so that different types of heartbeat could be implemented (preferably without creating external dependencies per discussion above). Thoughts welcome. F. > >> >>> >>> IMHO the safer way is to kick the host off the network by reprogramming >>> switches; so even if the qemu is actually alive it can't get anywhere. >>> >>> Dave >> >> Naturally some off-host STONITH is preferable, but that's not always >> available. A self-fencing mechanism right at the heart of the emulator >> can do the job without external hardware dependencies. > > Dave > >> Cheers, >> Felipe >> >>> >>> Felipe > >> Does something already exist for this purpose which could be used? >> Would a generic Qemu-fencing infrastructure be something of interest? > Dave > > >> Cheers, >> F. >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK >>> -- >>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] hw/isa: Introduce a CONFIG_ISA_SUPERIO switch for isa-superio.c
Patchew URL: https://patchew.org/QEMU/20190930150436.18162-1-th...@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190930150436.18162-1-th...@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v3 02/18] target/s390x: Add ilen to unwind data
On 30.09.19 18:15, Richard Henderson wrote: > On 9/30/19 8:42 AM, David Hildenbrand wrote: >>> Of course, but it's no different with this case than any other. If the >>> interrupt has already been handled, then we will simply restart the next TB >>> as >>> per normal. >> Yeah, I was mostly concerned that "the next TB" will be "the next >> instruction" and not "the original instruction again". > > Ah, well, that's easy. The next TB will start from psw.addr, and when we > cpu_loop_exit_restore(), psw.addr will be set to the current instruction. > > This is exactly the same as when we raise a PGM_ADDRESSING exception which > needs to be restarted after the kernel swaps in the page. > Now that I think about it, this makes perfect sense :) So it's mostly checking if there is an exception (like you said) and then doing a cpu_loop_exit_restore() (if I'm not wrong, but I'll have to look at the details). -- Thanks, David / dhildenb
Re: Thoughts on VM fence infrastructure
* Felipe Franciosi (fel...@nutanix.com) wrote: > > > > On Sep 30, 2019, at 5:03 PM, Dr. David Alan Gilbert > > wrote: > > > > * Felipe Franciosi (fel...@nutanix.com) wrote: > >> Hi David, > >> > >>> On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert > >>> wrote: > >>> > >>> * Felipe Franciosi (fel...@nutanix.com) wrote: > Heyall, > > We have a use case where a host should self-fence (and all VMs should > die) if it doesn't hear back from a heartbeat within a certain time > period. Lots of ideas were floated around where libvirt could take > care of killing VMs or a separate service could do it. The concern > with those is that various failures could lead to _those_ services > being unavailable and the fencing wouldn't be enforced as it should. > > Ultimately, it feels like Qemu should be responsible for this > heartbeat and exit (or execute a custom callback) on timeout. > >>> > >>> It doesn't feel doing it inside qemu would be any safer; something > >>> outside QEMU can forcibly emit a kill -9 and qemu *will* stop. > >> > >> The argument above is that we would have to rely on this external > >> service being functional. Consider the case where the host is > >> dysfunctional, with this service perhaps crashed and a corrupt > >> filesystem preventing it from restarting. The VMs would never die. > > > > Yeh that could fail. > > > >> It feels like a Qemu timer-driven heartbeat check and calls abort() / > >> exit() would be more reliable. Thoughts? > > > > OK, yes; perhaps using a timer_create and telling it to send a fatal > > signal is pretty solid; it would take the kernel to do that once it's > > set. > > I'm confused about why the kernel needs to be involved. If this is a > timer off the Qemu main loop, it can just check on the heartbeat > condition (which should be customisable) and call abort() if that's > not satisfied. If you agree on that I'd like to talk about how that > check could be made customisable. There are times when the main loop can get blocked even though the CPU threads can be running and can in some configurations perform IO even without the main loop (I think!). By setting a timer in the kernel that sends a signal to qemu, the kernel will send that signal however broken qemu is. > > > > > IMHO the safer way is to kick the host off the network by reprogramming > > switches; so even if the qemu is actually alive it can't get anywhere. > > > > Dave > > Naturally some off-host STONITH is preferable, but that's not always > available. A self-fencing mechanism right at the heart of the emulator > can do the job without external hardware dependencies. Dave > Cheers, > Felipe > > > > > > >> Felipe > >> > >>> > Does something already exist for this purpose which could be used? > Would a generic Qemu-fencing infrastructure be something of interest? > >>> Dave > >>> > >>> > Cheers, > F. > > >>> -- > >>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 00/11] RFC crypto/luks: encryption key managment using amend interface
On Fri, 2019-09-20 at 17:14 -0400, John Snow wrote: > > On 9/12/19 6:30 PM, Maxim Levitsky wrote: > > This patch series is continuation of my work to add encryption > > key managment to luks/qcow2 with luks. > > > > This is second version of this patch set. > > The changes are mostly addressing the review feedback, > > plus I tested (and fixed sadly) the somewhat ugly code > > that allows to still write share a raw luks device, > > while preveting the key managment from happening in this case, > > as it is unsafe. > > I added a new iotest dedicated to that as well. > > > > Best regards, > > Maxim Levitsky > > > > What branch is this based on? > It doesn't seem to apply to origin/master. > > --js Hi! Most of the refactoring patches are now on the master branch, (one patch was dropped due to me being blind :-(), so should I resend this patch series with the missing patch or wait for some review? At that stage I would like to hear about agreement/disagreement on the new APIs, and stuff like that. Best regards, Maxim Levitsky
[PATCH] target/sparc: Remove old TODO file
This file hasn't seen a real (non-trivial) update since 2008 anymore, so we can assume that it is pretty much out of date and nobody cares for it anymore. Let's simply remove it. Signed-off-by: Thomas Huth --- target/sparc/TODO | 88 --- 1 file changed, 88 deletions(-) delete mode 100644 target/sparc/TODO diff --git a/target/sparc/TODO b/target/sparc/TODO deleted file mode 100644 index b8c727e858..00 --- a/target/sparc/TODO +++ /dev/null @@ -1,88 +0,0 @@ -TODO-list: - -CPU common: -- Unimplemented features/bugs: - - Delay slot handling may fail sometimes (branch end of page, delay - slot next page) - - Atomical instructions - - CPU features should match real CPUs (also ASI selection) -- Optimizations/improvements: - - Condition code/branch handling like x86, also for FPU? - - Remove remaining explicit alignment checks - - Global register for regwptr, so that windowed registers can be - accessed directly - - Improve Sparc32plus addressing - - NPC/PC static optimisations (use JUMP_TB when possible)? (Is this - obsolete?) - - Synthetic instructions - - MMU model dependent on CPU model - - Select ASI helper at translation time (on V9 only if known) - - KQemu/KVM support for VM only - - Hardware breakpoint/watchpoint support - - Cache emulation mode - - Reverse-endian pages - - Faster FPU emulation - - Busy loop detection - -Sparc32 CPUs: -- Unimplemented features/bugs: - - Sun4/Sun4c MMUs - - Some V8 ASIs - -Sparc64 CPUs: -- Unimplemented features/bugs: - - Interrupt handling - - Secondary address space, other MMU functions - - Many V9/UA2005/UA2007 ASIs - - Rest of V9 instructions, missing VIS instructions - - IG/MG/AG vs. UA2007 globals - - Full hypervisor support - - SMP/CMT - - Sun4v CPUs - -Sun4: -- To be added - -Sun4c: -- A lot of unimplemented features -- Maybe split from Sun4m - -Sun4m: -- Unimplemented features/bugs: - - Hardware devices do not match real boards - - Floppy does not work - - CS4231: merge with cs4231a, add DMA - - Add cg6, bwtwo - - Arbitrary resolution support - - PCI for MicroSparc-IIe - - JavaStation machines - - SBus slot probing, FCode ROM support - - SMP probing support - - Interrupt routing does not match real HW - - SuSE 7.3 keyboard sometimes unresponsive - - Gentoo 2004.1 SMP does not work - - SS600MP ledma -> lebuffer - - Type 5 keyboard - - Less fixed hardware choices - - DBRI audio (Am7930) - - BPP parallel - - Diagnostic switch - - ESP PIO mode - -Sun4d: -- A lot of unimplemented features: - - SBI - - IO-unit -- Maybe split from Sun4m - -Sun4u: -- Unimplemented features/bugs: - - Interrupt controller - - PCI/IOMMU support (Simba, JIO, Tomatillo, Psycho, Schizo, Safari...) - - SMP - - Happy Meal Ethernet, flash, I2C, GPIO - - A lot of real machine types - -Sun4v: -- A lot of unimplemented features - - A lot of real machine types -- 2.18.1
Re: [PATCH 06/20] xics: Create sPAPR specific ICS subtype
On Mon, 30 Sep 2019 18:45:30 +1000 David Gibson wrote: > On Fri, Sep 27, 2019 at 06:05:56PM +0200, Greg Kurz wrote: > > On Thu, 26 Sep 2019 10:56:46 +1000 > > David Gibson wrote: > > > > > On Wed, Sep 25, 2019 at 10:55:35AM +0200, Cédric Le Goater wrote: > > > > On 25/09/2019 10:40, Greg Kurz wrote: > > > > > On Wed, 25 Sep 2019 16:45:20 +1000 > > > > > David Gibson wrote: > > > > > > > > > >> We create a subtype of TYPE_ICS specifically for sPAPR. For now all > > > > >> this > > > > >> does is move the setup of the PAPR specific hcalls and RTAS calls to > > > > >> the realize() function for this, rather than requiring the PAPR code > > > > >> to > > > > >> explicitly call xics_spapr_init(). In future it will have some more > > > > >> function. > > > > >> > > > > >> Signed-off-by: David Gibson > > > > >> --- > > > > > > > > > > LGTM, but for extra safety I would also introduce a SpaprIcsState > > > > > typedef > > > > > > > > why ? we have ICS_SPAPR() for the checks already. > > > > > > Eh.. using typedefs when we haven't actually extended a base type > > > isn't common QOM practice. Yes, it's not as typesafe as it could be, > > > but I'm not really inclined to go to the extra effort here. > > > > I'll soon need to extend the base type with a nr_servers field, > > Uh.. nr_servers doesn't seem like it belongs in the base ICS type. Of course ! I re-used the wording "extended a base type" of your sentence, that I understand as "a subtype extends a base type with some more data". I'm talking about the sPAPR ICS subtype here, not the base ICS type. > That really would conflict with the pnv usage where the ICS is > supposed to just represent the ICS, not the xics as a whole. If you > need nr_servers information here, it seems like pulling it via a > method in XICSFabric would make more sense. > > > and while here with an fd field as well in order to get rid of > > the ugly global in xics.c. I'll go to the extra effort :) > > That could go in the derived type. We already kind of conflate ICS > and XICS-as-a-whole for the PAPR subtype, and the KVM xics is PAPR > only anyway. > Exactly, so that's why I was thinking about adding nr_servers there, but it could go to XICSFabric as well I guess. pgpOkcfUN_KLp.pgp Description: OpenPGP digital signature
Re: Thoughts on VM fence infrastructure
> On Sep 30, 2019, at 5:03 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> Hi David, >> >>> On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert >>> wrote: >>> >>> * Felipe Franciosi (fel...@nutanix.com) wrote: Heyall, We have a use case where a host should self-fence (and all VMs should die) if it doesn't hear back from a heartbeat within a certain time period. Lots of ideas were floated around where libvirt could take care of killing VMs or a separate service could do it. The concern with those is that various failures could lead to _those_ services being unavailable and the fencing wouldn't be enforced as it should. Ultimately, it feels like Qemu should be responsible for this heartbeat and exit (or execute a custom callback) on timeout. >>> >>> It doesn't feel doing it inside qemu would be any safer; something >>> outside QEMU can forcibly emit a kill -9 and qemu *will* stop. >> >> The argument above is that we would have to rely on this external >> service being functional. Consider the case where the host is >> dysfunctional, with this service perhaps crashed and a corrupt >> filesystem preventing it from restarting. The VMs would never die. > > Yeh that could fail. > >> It feels like a Qemu timer-driven heartbeat check and calls abort() / >> exit() would be more reliable. Thoughts? > > OK, yes; perhaps using a timer_create and telling it to send a fatal > signal is pretty solid; it would take the kernel to do that once it's > set. I'm confused about why the kernel needs to be involved. If this is a timer off the Qemu main loop, it can just check on the heartbeat condition (which should be customisable) and call abort() if that's not satisfied. If you agree on that I'd like to talk about how that check could be made customisable. > > IMHO the safer way is to kick the host off the network by reprogramming > switches; so even if the qemu is actually alive it can't get anywhere. > > Dave Naturally some off-host STONITH is preferable, but that's not always available. A self-fencing mechanism right at the heart of the emulator can do the job without external hardware dependencies. Cheers, Felipe > > >> Felipe >> >>> Does something already exist for this purpose which could be used? Would a generic Qemu-fencing infrastructure be something of interest? >>> Dave >>> >>> Cheers, F. >>> -- >>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v3 04/25] error: auto propagated local_err
Am 30.09.2019 um 18:26 hat Vladimir Sementsov-Ogievskiy geschrieben: > 30.09.2019 19:00, Kevin Wolf wrote: > > Am 30.09.2019 um 17:19 hat Vladimir Sementsov-Ogievskiy geschrieben: > >> 30.09.2019 18:12, Kevin Wolf wrote: > >>> Am 24.09.2019 um 22:08 hat Vladimir Sementsov-Ogievskiy geschrieben: > Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of > functions with errp parameter. > >>> > >>> A bit of bike shedding, but FOO_BEGIN suggests to me that a FOO_END will > >>> follow. Can we find a different name, especially now that we won't use > >>> this macro in every function that uses an errp, so even the "errp > >>> function" part isn't really correct any more? > >>> > >>> How about ERRP_AUTO_PROPAGATE? > >> > >> I have an idea that with this macro we can (optionally) get the whole call > >> stack > >> of the error and print it to log, so it's good to give it more generic > >> name, not > >> limited to propagation.. > > > > Hm, what's the context for this feature? > > > > The obvious one where you want to have a stack trace is &error_abort, > > but that one crashes, so you get it automatically. If it's just a normal > > error (like a QAPI option contains an invalid value and some function > > down the call chain checks it), why would anyone want to know what the > > call chain in the QEMU code was? > > > > When I have bug from testers, call stack would be a lot more descriptive, > than just > an error message. > > We may add trace point which will print this information, so with disabled > trace point > - no extra output. But wouldn't it make much more sense then to optionally add this functionality to any trace point? I really don't see how this is related specifically to user-visible error messages. However, even if we decide that we want to have this in Error objects, wouldn't it make much more sense to use the real C stack trace and save it from the innermost error_set() using backtrace() or compiler built-ins rather than relying on an error_propagate() chain? Kevin
Re: Lockup with --accel tcg,thread=single
Peter Maydell writes:
> On Mon, 30 Sep 2019 at 14:17, Doug Gale wrote:
>>
>> I found a lockup in single threaded TCG, with OVMF bios, 16 CPUs.
>>
>> qemu-system-x86_64 --accel tcg,thread=single -smp cpus=16 -bios
>> /usr/share/ovmf/OVMF.fd
>>
>> Using Ubuntu 18.04 LTS, default gnome desktop. There is no guest OS,
>> there is no hard drive, just the OVMF firmware locks it up. (I
>> narrowed it down to this from a much larger repro)
>
>> Peter Maydell helped me bisect it in IRC.
>> Works fine at commit 1e8a98b53867f61
>> Fails at commit 9458a9a1df1a4c7
>>
>> MTTCG works fine all the way up to master.
>
> Thanks for this bug report. I've reproduced it and think
> I have figured out what is going on here.
>
> Commit 9458a9a1df1a4c719e245 is Paolo's commit that fixes the
> TCG-vs-dirty-bitmap race by having the thread which is
> doing a memory access wait for the vCPU thread to finish
> its current TB using a no-op run_on_cpu() operation.
>
> In the case of the hang the thread doing the memory access
> is the iothread, like this:
>
> #14 0x55c150c0a98c in run_on_cpu (cpu=0x55c153801c60,
> func=0x55c150bbb542 , data=...)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1205
> #15 0x55c150bbb58c in tcg_log_global_after_sync
> (listener=0x55c1538410c8) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/exec.c:2963
> #16 0x55c150c1fe18 in memory_global_after_dirty_log_sync () at
> /home/petmay01/linaro/qemu-from-laptop/qemu/memory.c:2598
> #17 0x55c150c1e6b8 in memory_region_snapshot_and_clear_dirty
> (mr=0x55c1541e82b0, addr=0, size=192, client=0)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/memory.c:2106
> #18 0x55c150c76c05 in vga_draw_graphic (s=0x55c1541e82a0, full_update=0)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/display/vga.c:1661
> #19 0x55c150c771c4 in vga_update_display (opaque=0x55c1541e82a0)
> at /home/petmay01/linaro/qemu-from-laptop/qemu/hw/display/vga.c:1785
> #20 0x55c151052a83 in graphic_hw_update (con=0x55c1536dfaa0) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/ui/console.c:268
> #21 0x55c151091490 in gd_refresh (dcl=0x55c1549af090) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/ui/gtk.c:484
> #22 0x55c151056571 in dpy_refresh (s=0x55c1542f9d90) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/ui/console.c:1629
> #23 0x55c1510527f0 in gui_update (opaque=0x55c1542f9d90) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/ui/console.c:206
> #24 0x55c1511ee67c in timerlist_run_timers
> (timer_list=0x55c15370c280) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/util/qemu-timer.c:592
> #25 0x55c1511ee726 in qemu_clock_run_timers
> (type=QEMU_CLOCK_REALTIME) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/util/qemu-timer.c:606
> #26 0x55c1511ee9e5 in qemu_clock_run_all_timers () at
> /home/petmay01/linaro/qemu-from-laptop/qemu/util/qemu-timer.c:692
> #27 0x55c1511ef181 in main_loop_wait (nonblocking=0) at
> /home/petmay01/linaro/qemu-from-laptop/qemu/util/main-loop.c:524
> #28 0x55c150db03fe in main_loop () at
> /home/petmay01/linaro/qemu-from-laptop/qemu/vl.c:1806
>
> run_on_cpu() adds the do_nothing function to a cpu queued-work list.
>
> However, the single-threaded TCG runloop qemu_tcg_rr_cpu_thread_fn()
> has the basic structure:
>
>while (1) {
>for (each vCPU) {
>run this vCPU for a timeslice;
>}
>qemu_tcg_rr_wait_io_event();
>}
>
> and it processes cpu work queues only in qemu_tcg_rr_wait_io_event().
> This is a problem, because the thing which causes us to stop running
> one vCPU when its timeslice ends and move on to the next is the
> tcg_kick_vcpu_timer -- and the iothread will never process that timer
> and kick the vcpu because it is currently blocked in run_on_cpu() !
>
> Not sure currently what the best fix is here.
We seem to be repeating ourselves because:
a8efa60633575a2ee4dbf807a71cb44d44b0e0f8
Author: Paolo Bonzini
AuthorDate: Wed Nov 14 12:36:57 2018 +0100
cpus: run work items for all vCPUs if single-threaded
However looking at the commit I can still see we have the problem of not
advancing to the next vCPU if the kick timer (or some other event)
doesn't bring the execution to an exit. I suspect you could get this in
Linux but it's probably sufficiently busy to ensure vCPUs are always
exiting for some reason or another.
So options I can think of so far are:
1. Kick 'em all when not inter-vCPU
Something like this untested patch...
--8<---cut here---start->8---
1 file changed, 17 insertions(+), 5 deletions(-)
cpus.c | 22 +-
modified cpus.c
@@ -949,8 +949,8 @@ static inline int64_t qemu_tcg_next_kick(void)
return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + TCG_KICK_PERIOD;
}
-/* Kick the currently round-robin scheduled vCPU */
-static void qemu_cpu_kick_rr_cpu(void)
+/* Kick the currently round-robin scheduled vCPU to next */
+static void qemu_cpu_kick_rr_
Re: [PATCH v3 04/25] error: auto propagated local_err
30.09.2019 19:00, Kevin Wolf wrote: > Am 30.09.2019 um 17:19 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 30.09.2019 18:12, Kevin Wolf wrote: >>> Am 24.09.2019 um 22:08 hat Vladimir Sementsov-Ogievskiy geschrieben: Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of functions with errp parameter. >>> >>> A bit of bike shedding, but FOO_BEGIN suggests to me that a FOO_END will >>> follow. Can we find a different name, especially now that we won't use >>> this macro in every function that uses an errp, so even the "errp >>> function" part isn't really correct any more? >>> >>> How about ERRP_AUTO_PROPAGATE? >> >> I have an idea that with this macro we can (optionally) get the whole call >> stack >> of the error and print it to log, so it's good to give it more generic name, >> not >> limited to propagation.. > > Hm, what's the context for this feature? > > The obvious one where you want to have a stack trace is &error_abort, > but that one crashes, so you get it automatically. If it's just a normal > error (like a QAPI option contains an invalid value and some function > down the call chain checks it), why would anyone want to know what the > call chain in the QEMU code was? > When I have bug from testers, call stack would be a lot more descriptive, than just an error message. We may add trace point which will print this information, so with disabled trace point - no extra output. -- Best regards, Vladimir
Re: gdbstub and gbd segfaults on different instructions in user space emulation
On Mon, 30 Sep 2019 at 16:57, Libo Zhou wrote: > I am encountering segmentation fault while porting my custom ISA to QEMU. My > custom ISA is VERY VERY simple, it only changes the [31:26] opcode field of > LW and SW instructions. The link has my very simple implementation: > https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06976.html > I have tried 2 ways of debugging it. > Firstly, I connected gdb-multiarch to gdbstub, and I single-stepped the > instructions in my ELF. Immediately after the LW instruction, the segfault > was thrown. I observed the memory location using 'x' command and found that > at least my SW instruction was implemented correctly. > Secondly, I used gdb to directly debug QEMU. I set the breakpoint at function > in translate.c:decode_opc. Pressing 'c' should have the same effect as > single-stepping instruction in gdbstub. However, the segmentation fault > wasn't thrown after LW. It was instead thrown after the 'nop' after 'jr r31' > in the objdump. (1) If you're debugging the QEMU JIT itself, then you're probably better off using QEMU's logging facilities (under the -d option) rather than the gdbstub. The gdbstub is good if you're sure that QEMU is basically functional and want to debug your guest, but if you suspect bugs in QEMU itself then it can confuse you. The -d debug logging is at a much lower level, which makes it a better guide to what QEMU is really doing, though it is also trickier to interpret. (2) No, breakpointing on decode_opc is not the same as singlestepping an instruction in gdb. This is a really important concept in QEMU (and JITs in general) and if you don't understand it you're going to be very confused. A JIT has two phases: (a) "translate time", when we take a block of guest instructions and generate host machine code for them (b) "execution time", when we execute one or more of the blocks of host machine code that we wrote at translate time QEMU calls the blocks it works with "translation blocks", and usually it will put multiple guest instructions into each TB; a TB usually stops after a guest branch instructions. (You can ask QEMU to put just one guest instruction into a TB using the -singlestep command line option -- this is sometimes useful when debugging.) So if you put a breakpoint on decode_opc you'll see it is hit for every instruction in the TB, which for the TB starting at "00400090 " will be every instruction up to and including the 'nop' in the delay slot of the 'jr'. Once the whole TB is translated, *then* we will execute it. It's only at execute time that we perform the actual operations on the guest CPU that the instructions require. If the segfault is because we think the guest has made a bad memory access, we'll generate it here. If the segfault is an actual crash in QEMU itself, it will happen here if the bug is one that happens at execution time. Note that the -d logging will distinguish between things that happen at translate time (which is when the in_asm, op, out_asm etc logging is printed) and things that happen at execution time (which is when cpu, exec, int, etc logs are printed). thanks -- PMM
Re: [RFC] cpu_map: Remove pconfig from Icelake-Server CPU model
On Mon, Sep 30, 2019 at 17:16:27 +0200, Paolo Bonzini wrote: > On 30/09/19 16:31, Hu, Robert wrote: > >> This might be a problem if there are plans to eventually make KVM support > >> pconfig, though. Paolo, Robert, are there plans to support pconfig in KVM > >> in the > >> future? > > [Robert Hoo] > > Thanks Eduardo for efforts in resolving this issue, introduced from my > > Icelake CPU > > model patch. > > I've no idea about PCONFIG's detail and plan. Let me sync with Huang, Kai > > and answer > > you soon. > > It's really, really unlikely. It's possible that some future processor > overloads PCONFIG in such a way that it will become virtualizable, but > not IceLake. I guess, the likelihood of this happening would be similar to reintroducing other features, such as osxsave or ospke, right? > Would it make sense for libvirt to treat absent CPU flags as "default > off" during migration, so that it can leave out the flag in the command > line if it's off? If it's on, libvirt would pass pconfig=on as usual. > This is a variant of [2], but more generally applicable: > > > [2] However starting a domain with Icelake-Server so that it can be > > migrated or saved/restored on QEMU in 3.1.1 and 4.0.0 would be > > impossible. This can be solved by a different hack, which would drop > > pconfig=off from QEMU command line. The domain XML does not contain a complete list of all CPU features. Features which are implicitly included in a CPU model are not listed in the XML. Count in the differences in libvirt's vs QEMU's definitions of a particular CPU model and you can see feat=off cannot be mechanically dropped from the command line as the CPU model itself could turn it on by default and thus feat=off is not redundant. Jirka
Re: [PATCH v3 02/18] target/s390x: Add ilen to unwind data
On 9/30/19 8:42 AM, David Hildenbrand wrote: >> Of course, but it's no different with this case than any other. If the >> interrupt has already been handled, then we will simply restart the next TB >> as >> per normal. > Yeah, I was mostly concerned that "the next TB" will be "the next > instruction" and not "the original instruction again". Ah, well, that's easy. The next TB will start from psw.addr, and when we cpu_loop_exit_restore(), psw.addr will be set to the current instruction. This is exactly the same as when we raise a PGM_ADDRESSING exception which needs to be restarted after the kernel swaps in the page. r~
Re: [PATCH v3 04/25] error: auto propagated local_err
Am 30.09.2019 um 17:19 hat Vladimir Sementsov-Ogievskiy geschrieben: > 30.09.2019 18:12, Kevin Wolf wrote: > > Am 24.09.2019 um 22:08 hat Vladimir Sementsov-Ogievskiy geschrieben: > >> Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of > >> functions with errp parameter. > > > > A bit of bike shedding, but FOO_BEGIN suggests to me that a FOO_END will > > follow. Can we find a different name, especially now that we won't use > > this macro in every function that uses an errp, so even the "errp > > function" part isn't really correct any more? > > > > How about ERRP_AUTO_PROPAGATE? > > I have an idea that with this macro we can (optionally) get the whole call > stack > of the error and print it to log, so it's good to give it more generic name, > not > limited to propagation.. Hm, what's the context for this feature? The obvious one where you want to have a stack trace is &error_abort, but that one crashes, so you get it automatically. If it's just a normal error (like a QAPI option contains an invalid value and some function down the call chain checks it), why would anyone want to know what the call chain in the QEMU code was? Kevin
Re: Thoughts on VM fence infrastructure
* Felipe Franciosi (fel...@nutanix.com) wrote: > Hi David, > > > On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert > > wrote: > > > > * Felipe Franciosi (fel...@nutanix.com) wrote: > >> Heyall, > >> > >> We have a use case where a host should self-fence (and all VMs should > >> die) if it doesn't hear back from a heartbeat within a certain time > >> period. Lots of ideas were floated around where libvirt could take > >> care of killing VMs or a separate service could do it. The concern > >> with those is that various failures could lead to _those_ services > >> being unavailable and the fencing wouldn't be enforced as it should. > >> > >> Ultimately, it feels like Qemu should be responsible for this > >> heartbeat and exit (or execute a custom callback) on timeout. > > > > It doesn't feel doing it inside qemu would be any safer; something > > outside QEMU can forcibly emit a kill -9 and qemu *will* stop. > > The argument above is that we would have to rely on this external > service being functional. Consider the case where the host is > dysfunctional, with this service perhaps crashed and a corrupt > filesystem preventing it from restarting. The VMs would never die. Yeh that could fail. > It feels like a Qemu timer-driven heartbeat check and calls abort() / > exit() would be more reliable. Thoughts? OK, yes; perhaps using a timer_create and telling it to send a fatal signal is pretty solid; it would take the kernel to do that once it's set. IMHO the safer way is to kick the host off the network by reprogramming switches; so even if the qemu is actually alive it can't get anywhere. Dave > Felipe > > > > >> Does something already exist for this purpose which could be used? > >> Would a generic Qemu-fencing infrastructure be something of interest? > > Dave > > > > > >> Cheers, > >> F. > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
gdbstub and gbd segfaults on different instructions in user space emulation
Hi all, I am encountering segmentation fault while porting my custom ISA to QEMU. My custom ISA is VERY VERY simple, it only changes the [31:26] opcode field of LW and SW instructions. The link has my very simple implementation: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06976.html Below is the objdump of the main part of my simple ELF. The dots are the automatically generated nop's by the compiler. 00400090 : 400090: 23bdffe0addir29,r29,-32 400094: 7fbe001cswr30,28(r29. ... 4000a0: 03a0f021addur30,r29,r0 4000a4: 20020001lir2,1# int a = 1; 4000a8: 7fc20010swr2,16(r30) ... 4000b4: 20020002lir2,2# int b = 2; 4000b8: 7fc2000cswr2,12(r30) ... 4000c4: 5fc30010lwr3,16(r30) 4000c8: nop 4000cc: 5fc2000clwr2,12(r30) ... 4000d8: 00621020addr2,r3,r2# int c = a + b; 4000dc: 7fc20008swr2,8(r30) ... 4000e8: 1021addur2,r0,r0 4000ec: 03c0e821addur29,r30,r0 4000f0: 5fbe001clwr30,28(r29) 4000f4: 23bd0020addir29,r29,32 4000f8: 03e8jrr31 4000fc: nop ... The code below gives me segfault: $ ./qemu-mipsel -cpu mycpu testprogram I have tried 2 ways of debugging it. Firstly, I connected gdb-multiarch to gdbstub, and I single-stepped the instructions in my ELF. Immediately after the LW instruction, the segfault was thrown. I observed the memory location using 'x' command and found that at least my SW instruction was implemented correctly. Secondly, I used gdb to directly debug QEMU. I set the breakpoint at function in translate.c:decode_opc. Pressing 'c' should have the same effect as single-stepping instruction in gdbstub. However, the segmentation fault wasn't thrown after LW. It was instead thrown after the 'nop' after 'jr r31' in the objdump. At this point, I am really stuck. I have spent a long time on this, but I just can't figure out what is going wrong here. If anyone can help me out I would really appreciate it. Cheers, Libo Zhou
Re: Thoughts on VM fence infrastructure
Hi David, > On Sep 30, 2019, at 3:29 PM, Dr. David Alan Gilbert > wrote: > > * Felipe Franciosi (fel...@nutanix.com) wrote: >> Heyall, >> >> We have a use case where a host should self-fence (and all VMs should >> die) if it doesn't hear back from a heartbeat within a certain time >> period. Lots of ideas were floated around where libvirt could take >> care of killing VMs or a separate service could do it. The concern >> with those is that various failures could lead to _those_ services >> being unavailable and the fencing wouldn't be enforced as it should. >> >> Ultimately, it feels like Qemu should be responsible for this >> heartbeat and exit (or execute a custom callback) on timeout. > > It doesn't feel doing it inside qemu would be any safer; something > outside QEMU can forcibly emit a kill -9 and qemu *will* stop. The argument above is that we would have to rely on this external service being functional. Consider the case where the host is dysfunctional, with this service perhaps crashed and a corrupt filesystem preventing it from restarting. The VMs would never die. It feels like a Qemu timer-driven heartbeat check and calls abort() / exit() would be more reliable. Thoughts? Felipe > >> Does something already exist for this purpose which could be used? >> Would a generic Qemu-fencing infrastructure be something of interest? > Dave > > >> Cheers, >> F. >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v7 0/9] linux-user: strace improvements
On 9/15/19 11:39 PM, Philippe Mathieu-Daudé wrote: > Hi Laurent, > > Few patches I'v been writting while trying to figure out this issue: > http://lists.nongnu.org/archive/html/qemu-arm/2018-01/msg00514.html > > As usual with linux-user files, this series will trigger some checkpatch > benign warnings. > > Regards, > > Phil. > > Since v6: > - Use ABI types in sockaddr > > Since v5: > - dropped 'Verify recvfrom(addr)' since failing LTP testsuite (see [1]) > - also define print_sockfd() for bind() (patches #6 and #7) > > Since v4: > - rebased on master (no change) > > Since v3: > - addressed Laurent comments > - added print_sockfd() > - removed the print_sockaddr_ptr() patch, also the two > getsockname()/recvfrom() patches for after 3.0. > > Since v2: > - display invalid pointer in print_timeval() and print_timezone() > - do not display gettimeofday() arguments > > Since v1: > - addressed Laurent comments > - added 'last' argument to print_sockaddr() > - reordered series, so patches already correct can get applied directly > - dropped "linux-user/syscall: simplify recvfrom()" for now > > v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg05855.html > v2: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08216.html > v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg00411.html > v5: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02067.html > v6: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg01346.html > [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02807.html Ping?
Re: [PATCH v4 09/18] target/s390x: Return exception from mmu_translate
On 27.09.19 21:39, Richard Henderson wrote:
> Do not raise the exception directly within mmu_translate,
> but pass it back so that caller may do so.
>
> Signed-off-by: Richard Henderson
> ---
> target/s390x/internal.h| 2 +-
> target/s390x/excp_helper.c | 4 ++--
> target/s390x/mem_helper.c | 13 +++---
> target/s390x/mmu_helper.c | 49 +++---
> 4 files changed, 32 insertions(+), 36 deletions(-)
>
> diff --git a/target/s390x/internal.h b/target/s390x/internal.h
> index c4388aaf23..c993c3ef40 100644
> --- a/target/s390x/internal.h
> +++ b/target/s390x/internal.h
> @@ -360,7 +360,7 @@ void probe_write_access(CPUS390XState *env, uint64_t
> addr, uint64_t len,
>
> /* mmu_helper.c */
> int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t
> asc,
> - target_ulong *raddr, int *flags, bool exc);
> + target_ulong *raddr, int *flags, uint64_t *tec);
> int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
> target_ulong *addr, int *flags, uint64_t *tec);
>
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index 906b87c071..6a0728b65f 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -140,8 +140,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> if (!(env->psw.mask & PSW_MASK_64)) {
> vaddr &= 0x7fff;
> }
> -fail = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot,
> true);
> -excp = 0; /* exception already raised */
> +excp = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot,
> &tec);
> +fail = excp;
> } else if (mmu_idx == MMU_REAL_IDX) {
> /* 31-Bit mode */
> if (!(env->psw.mask & PSW_MASK_64)) {
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 7d2a652823..e15aa296dd 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -2364,8 +2364,8 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> CPUState *cs = env_cpu(env);
> uint32_t cc = 0;
> uint64_t asc = env->psw.mask & PSW_MASK_ASC;
> -uint64_t ret;
> -int old_exc, flags;
> +uint64_t ret, tec;
> +int old_exc, flags, exc;
>
> /* XXX incomplete - has more corner cases */
> if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
> @@ -2373,7 +2373,14 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> }
>
> old_exc = cs->exception_index;
> -if (mmu_translate(env, addr, 0, asc, &ret, &flags, true)) {
> +exc = mmu_translate(env, addr, 0, asc, &ret, &flags, &tec);
> +if (exc) {
> +/*
> + * We don't care about ILEN or TEC, as we're not going to
> + * deliver the exception -- thus resetting exception_index below.
> + * TODO: clean this up.
> + */
> +trigger_pgm_exception(env, exc, ILEN_UNWIND);
> cc = 3;
> }
> if (cs->exception_index == EXCP_PGM) {
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index aa8712221e..8ea1c95549 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -369,17 +369,15 @@ static void mmu_handle_skey(target_ulong addr, int rw,
> int *flags)
> * @return 0 if the translation was successful, -1 if a fault occurred
> */
> int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t
> asc,
> - target_ulong *raddr, int *flags, bool exc)
> + target_ulong *raddr, int *flags, uint64_t *tec)
> {
> -/* Code accesses have an undefined ilc, let's use 2 bytes. */
> -const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
> -uint64_t tec = (vaddr & TARGET_PAGE_MASK) | (asc >> 46) |
> - (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
> uint64_t asce;
> int r;
>
> -
> +*tec = (vaddr & TARGET_PAGE_MASK) | (asc >> 46) |
> +(rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
> *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> +
> if (is_low_address(vaddr & TARGET_PAGE_MASK) && lowprot_enabled(env,
> asc)) {
> /*
> * If any part of this page is currently protected, make sure the
> @@ -391,12 +389,9 @@ int mmu_translate(CPUS390XState *env, target_ulong
> vaddr, int rw, uint64_t asc,
> */
> *flags |= PAGE_WRITE_INV;
> if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
> -if (exc) {
> -/* LAP sets bit 56 */
> -tec |= 0x80;
> -trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
> -}
> -return -EACCES;
> +/* LAP sets bit 56 */
> +*tec |= 0x80;
> +return PGM_PROTECTION;
> }
> }
>
> @@ -426,30 +421,21 @@ int mmu_translate(CPUS390XState *env, target_ulong
> vaddr, int rw, uint64_t asc,
> /* p
Re: [PATCH v4 18/18] target/s390x: Remove ILEN_UNWIND
On 27.09.19 21:39, Richard Henderson wrote: > This setting is no longer used. > > Signed-off-by: Richard Henderson > --- > target/s390x/cpu.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 686cbe41e0..fe1bf746f3 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -803,8 +803,6 @@ int cpu_s390x_signal_handler(int host_signum, void > *pinfo, void *puc); > void s390_crw_mchk(void); > void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr, > uint32_t io_int_parm, uint32_t io_int_word); > -/* instruction length set by unwind info */ > -#define ILEN_UNWIND 0 > #define RA_IGNORED 0 > void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra); > /* service interrupts are floating therefore we must not pass an cpustate */ > Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
