[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 02/13] spapr/xive: add hcall support when under
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH v2 02/13] spapr/xive: add hcall support when under KVM |
Date: |
Thu, 14 Mar 2019 13:11:41 +1100 |
User-agent: |
Mutt/1.11.3 (2019-02-01) |
On Wed, Mar 13, 2019 at 11:43:54AM +0100, Cédric Le Goater wrote:
> On 3/12/19 11:26 AM, David Gibson wrote:
> > On Mon, Mar 11, 2019 at 06:32:05PM +0100, Cédric Le Goater wrote:
> >> On 2/26/19 12:22 AM, David Gibson wrote:
> >>> On Fri, Feb 22, 2019 at 02:13:11PM +0100, Cédric Le Goater wrote:
> > [snip]
> >>>> +void kvmppc_xive_set_source_config(sPAPRXive *xive, uint32_t lisn,
> >>>> XiveEAS *eas,
> >>>> + Error **errp)
> >>>> +{
> >>>> + uint32_t end_idx;
> >>>> + uint32_t end_blk;
> >>>> + uint32_t eisn;
> >>>> + uint8_t priority;
> >>>> + uint32_t server;
> >>>> + uint64_t kvm_src;
> >>>> + Error *local_err = NULL;
> >>>> +
> >>>> + /*
> >>>> + * No need to set a MASKED source, this is the default state after
> >>>> + * reset.
> >>>
> >>> I don't quite follow this comment, why is there no need to call a
> >>> MASKED source?
> >>
> >> because MASKED is the default state in which KVM initializes the IRQ. I
> >> will
> >> clarify.
> >
> > I believe it's possible - though rare - to process an incoming
> > migration on an established VM which isn't in fresh reset state. So
> > it's best not to rely on that.
> >
> >>>> +static void xive_esb_trigger(XiveSource *xsrc, int srcno)
> >>>> +{
> >>>> + unsigned long addr = (unsigned long) xsrc->esb_mmap +
> >>>> + xive_source_esb_page(xsrc, srcno);
> >>>> +
> >>>> + *((uint64_t *) addr) = 0x0;
> >>>> +}
> >>>
> >>> Also.. aren't some of these register accesses likely to need memory
> >>> barriers?
> >>
> >> AIUI, these are CI pages. So we shouldn't need barriers.
> >
> > CI doesn't negate the need for barriers, althugh it might change the
> > type you need. At the very least you need a compiler barrier to stop
> > it re-ordering the access, but you can also have in-cpu store and load
> > queues.
> >
>
> ok. So I will need to add some smp_r/wmb()
No, smp_[rw]mb() is for cases where it's strictly about cpu vs. cpu
ordering. Here it's cpu vs. IO ordering so you need plain [rw]mb().
--
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