On 3/14/19 3:11 AM, David Gibson wrote:
> 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().
I don't see any in QEMU ?
C.
