[Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()

Yi Min Zhao posted 3 patches 8 years ago
There is a newer version of this series
[Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()
Posted by Yi Min Zhao 8 years ago
When registering ioat, pba should be comprised of leftmost 52 bits and
rightmost 12 binary zeros, and pal should be comprised of leftmost 52
bits and right most 12 binary ones. Let's fixup this.

Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
---
 hw/s390x/s390-pci-inst.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 997a9cc2e9..3fcc330fe3 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -865,6 +865,8 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
     uint8_t dt = (g_iota >> 2) & 0x7;
     uint8_t t = (g_iota >> 11) & 0x1;
 
+    pba &= ~0xfff;
+    pal |= 0xfff;
     if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
         s390_program_interrupt(env, PGM_OPERAND, 6, ra);
         return -EINVAL;
-- 
2.14.3 (Apple Git-98)


Re: [Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()
Posted by Cornelia Huck 8 years ago
On Tue, 30 Jan 2018 10:47:15 +0100
Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:

> When registering ioat, pba should be comprised of leftmost 52 bits and
> rightmost 12 binary zeros, and pal should be comprised of leftmost 52
> bits and right most 12 binary ones. Let's fixup this.
> 
> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-pci-inst.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 997a9cc2e9..3fcc330fe3 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -865,6 +865,8 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
>      uint8_t dt = (g_iota >> 2) & 0x7;
>      uint8_t t = (g_iota >> 11) & 0x1;
>  
> +    pba &= ~0xfff;
> +    pal |= 0xfff;
>      if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
>          s390_program_interrupt(env, PGM_OPERAND, 6, ra);
>          return -EINVAL;

It seems like pba and pal are part of the fib, which in turn seems to
be provided by the caller. Is that correct? If yes, is it valid for
them to not have the rightmost 12 bits as 0s resp. 1s?

(Probably answered in the architecture, I know. Might make sense to be
a tad more explicit in the description.)

Re: [Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()
Posted by Pierre Morel 8 years ago
On 31/01/2018 12:44, Cornelia Huck wrote:
> On Tue, 30 Jan 2018 10:47:15 +0100
> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>
>> When registering ioat, pba should be comprised of leftmost 52 bits and
>> rightmost 12 binary zeros, and pal should be comprised of leftmost 52
>> bits and right most 12 binary ones. Let's fixup this.
>>
>> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>> ---
>>   hw/s390x/s390-pci-inst.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>> index 997a9cc2e9..3fcc330fe3 100644
>> --- a/hw/s390x/s390-pci-inst.c
>> +++ b/hw/s390x/s390-pci-inst.c
>> @@ -865,6 +865,8 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
>>       uint8_t dt = (g_iota >> 2) & 0x7;
>>       uint8_t t = (g_iota >> 11) & 0x1;
>>   
>> +    pba &= ~0xfff;
>> +    pal |= 0xfff;
>>       if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
>>           s390_program_interrupt(env, PGM_OPERAND, 6, ra);
>>           return -EINVAL;
> It seems like pba and pal are part of the fib, which in turn seems to
> be provided by the caller. Is that correct? If yes, is it valid for
> them to not have the rightmost 12 bits as 0s resp. 1s?
>
> (Probably answered in the architecture, I know. Might make sense to be
> a tad more explicit in the description.)
>
Yes it is, only word6 and the bits 0-19 of word 7 are used for PAL and
the zPCI facility treats the right most 12 bits of the PAL as containing 
ones.

For PBA words 4 and 0-19 bits of word 5 with 12 0 append on the right 
provides the PBA.

The lower 12 bits of words 5 and 7 of the FIB are ignored by the facility.

@Yi Min: may be add the last sentence to the commit message.

@Conny: Is it clearer?

Regards,

Pierre


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany


Re: [Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()
Posted by Cornelia Huck 8 years ago
On Thu, 1 Feb 2018 12:33:01 +0100
Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:

> On 31/01/2018 12:44, Cornelia Huck wrote:
> > On Tue, 30 Jan 2018 10:47:15 +0100
> > Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
> >  
> >> When registering ioat, pba should be comprised of leftmost 52 bits and
> >> rightmost 12 binary zeros, and pal should be comprised of leftmost 52
> >> bits and right most 12 binary ones. Let's fixup this.
> >>
> >> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> >> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> >> ---
> >>   hw/s390x/s390-pci-inst.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> >> index 997a9cc2e9..3fcc330fe3 100644
> >> --- a/hw/s390x/s390-pci-inst.c
> >> +++ b/hw/s390x/s390-pci-inst.c
> >> @@ -865,6 +865,8 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
> >>       uint8_t dt = (g_iota >> 2) & 0x7;
> >>       uint8_t t = (g_iota >> 11) & 0x1;
> >>   
> >> +    pba &= ~0xfff;
> >> +    pal |= 0xfff;
> >>       if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
> >>           s390_program_interrupt(env, PGM_OPERAND, 6, ra);
> >>           return -EINVAL;  
> > It seems like pba and pal are part of the fib, which in turn seems to
> > be provided by the caller. Is that correct? If yes, is it valid for
> > them to not have the rightmost 12 bits as 0s resp. 1s?
> >
> > (Probably answered in the architecture, I know. Might make sense to be
> > a tad more explicit in the description.)
> >  
> Yes it is, only word6 and the bits 0-19 of word 7 are used for PAL and
> the zPCI facility treats the right most 12 bits of the PAL as containing 
> ones.
> 
> For PBA words 4 and 0-19 bits of word 5 with 12 0 append on the right 
> provides the PBA.
> 
> The lower 12 bits of words 5 and 7 of the FIB are ignored by the facility.
> 
> @Yi Min: may be add the last sentence to the commit message.
> 
> @Conny: Is it clearer?

Yes, adding the last sentence makes it clearer. Thanks!

Re: [Qemu-devel] [PATCH 3/3] s390x/pci: use the right pal and pba in reg_ioat()
Posted by Yi Min Zhao 8 years ago

在 2018/2/1 下午8:02, Cornelia Huck 写道:
> On Thu, 1 Feb 2018 12:33:01 +0100
> Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:
>
>> On 31/01/2018 12:44, Cornelia Huck wrote:
>>> On Tue, 30 Jan 2018 10:47:15 +0100
>>> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>>>   
>>>> When registering ioat, pba should be comprised of leftmost 52 bits and
>>>> rightmost 12 binary zeros, and pal should be comprised of leftmost 52
>>>> bits and right most 12 binary ones. Let's fixup this.
>>>>
>>>> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
>>>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>>> ---
>>>>    hw/s390x/s390-pci-inst.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
>>>> index 997a9cc2e9..3fcc330fe3 100644
>>>> --- a/hw/s390x/s390-pci-inst.c
>>>> +++ b/hw/s390x/s390-pci-inst.c
>>>> @@ -865,6 +865,8 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
>>>>        uint8_t dt = (g_iota >> 2) & 0x7;
>>>>        uint8_t t = (g_iota >> 11) & 0x1;
>>>>    
>>>> +    pba &= ~0xfff;
>>>> +    pal |= 0xfff;
>>>>        if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
>>>>            s390_program_interrupt(env, PGM_OPERAND, 6, ra);
>>>>            return -EINVAL;
>>> It seems like pba and pal are part of the fib, which in turn seems to
>>> be provided by the caller. Is that correct? If yes, is it valid for
>>> them to not have the rightmost 12 bits as 0s resp. 1s?
>>>
>>> (Probably answered in the architecture, I know. Might make sense to be
>>> a tad more explicit in the description.)
>>>   
>> Yes it is, only word6 and the bits 0-19 of word 7 are used for PAL and
>> the zPCI facility treats the right most 12 bits of the PAL as containing
>> ones.
>>
>> For PBA words 4 and 0-19 bits of word 5 with 12 0 append on the right
>> provides the PBA.
>>
>> The lower 12 bits of words 5 and 7 of the FIB are ignored by the facility.
>>
>> @Yi Min: may be add the last sentence to the commit message.
>>
>> @Conny: Is it clearer?
> Yes, adding the last sentence makes it clearer. Thanks!
>
>
OK. Thanks!