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