Re: [Qemu-devel] [PATCH] spapr-pci: remove io ports workaround

[Alexey Kardashevskiy]

On 03.01.2014 9:09, Alexander Graf wrote:
> 
> On 02.01.2014, at 23:08, Alexey Kardashevskiy <address@hidden> wrote:
> 
>> On 01/03/2014 08:04 AM, Alexander Graf wrote:
>>>
>>> On 11.12.2013, at 07:47, Alexey Kardashevskiy <address@hidden> wrote:
>>>
>>>> On 12/10/2013 06:47 PM, Greg Kurz wrote:
>>>>> On Tue, 10 Dec 2013 13:43:05 +1100
>>>>> Alexey Kardashevskiy <address@hidden> wrote:
>>>>>> On 12/10/2013 03:33 AM, Greg Kurz wrote:
>>>>>>> In the past, IO space could not be mapped into the memory address space
>>>>>>> so we introduced a workaround for that. Nowadays it does not look
>>>>>>> necessary so we can remove the workaround and make sPAPR PCI
>>>>>>> configuration simplier.
>>>>>>>
>>>>>>> This workaround has also an evil side effect with virtio devices:
>>>>>>> because all PHBs have their .io region at the same address, the devices
>>>>>>> get mapped in the .io-alias region of every PHB (AKA. mapped multiple
>>>>>>> times). This breaks the ioeventfd feature and causes qemu to abort()
>>>>>>> when running with KVM and asking for more than one PHB:
>>>>>>>
>>>>>>> $ qemu-system-ppc64 -machine type=pseries,accel=kvm -smp 1 -m 4G \
>>>>>>> -hda /local/greg/images/fedora-be.qcow2 \
>>>>>>> -device
>>>>>>> virtio-9p-pci,fsdev=fsdev0,mount_tag=share,bus=pci,ioeventfd=on \
>>>>>>> -fsdev local,security_model=none,id=fsdev0,path=$HOME/share1 \ -device
>>>>>>> spapr-pci-host-bridge,index=15 kvm_mem_ioeventfd_add: error adding
>>>>>>> ioeventfd: File exists Aborted
>>>>>>>
>>>>>>> This will prevent to use virtio and VFIO passthrough at the same time,
>>>>>>> since VFIO needs a dedicated PHB to work on ppc.
>>>>>>>
>>>>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>>>>
>>>>>>
>>>>>> I have not seen this version yet so please remove me from "SOB". The 
>>>>>> patch
>>>>>> you replied to was eventually reworked and went to upstream as
>>>>>> 66aab867cedd2a2d81b4d64eff7c3e0f6f272bbf
>>>>>>
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>> I agree you have not seen this version yet... The patch I replied to was 
>>>>> my
>>>>> primary source of inspiration and contains these bits, hence the SOB. 
>>>>> Anyway, the SOB is now removed until you decide to add one yourself. :)
>>>>>
>>>>>> This one might be correct too but I want to try this first :)
>>>>>>
>>>>>
>>>>> Well, I hope it is. Please try it.
>>>>
>>>>
>>>> Yep. Tried. Looks good, did not break a thing as far as I can tell, even
>>>> VGA works :)
>>>>
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>
>>> Thanks, applied to ppc-next.
>>
>>
>> Please read the rest of this thread. It does not visibly break things but
>> with this patch QEMU starts calling unassigned_mem_accepts() (normally
>> silent) which is not a good sign.
> 
> Oops, I thought your comment meant this was fixed. I took it off the queue 
> again :).

Thanks :)

And I have another bug like that - SPR registers are never
saved/restored via KVM's set-one-reg mechanism (with the loop through
all 1024 SPRs) because one_reg_id in SPR's descriptor is never
initialized (the macro misses assignment) and I made a patch for that
but cannot post it as some registers (AMR?) breaks HV KVM :)


-- 
With best regards

Alexey Kardashevskiy -- icq: 52150396