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()
Thanks,
C.
