Re: [Qemu-devel] [PATCH v2 4/7] s390x/pci: rework PCI STORE BLOCK

[Yi Min Zhao]

在 2017/11/22 上午2:07, Pierre Morel 写道:
> On 21/11/2017 11:42, Cornelia Huck wrote:
> > On Thu, 16 Nov 2017 18:51:52 +0100
> > Pierre Morel <address@hidden> wrote:
> >
> > > Enhance the fault detection.
> > >
> > > Fixup the precedence to check the destination path existance
> > > before checking for the source accessibility.
> > >
> > > Add the maxstbl entry to both the Query PCI Function Group
> > > response and the PCIBusDevice structure.
> > >
> > > Initialize the maxstbl to 128 per default until we get
> > > the actual data from the hardware.
> > >
> > > Signed-off-by: Pierre Morel <address@hidden>
> > > Reviewed-by: Yi Min Zhao <address@hidden>
> > > ---
> > >   hw/s390x/s390-pci-bus.h  |  1 +
> > >   hw/s390x/s390-pci-inst.c | 62 
> > > +++++++++++++++++++++++++++++-------------------
> > >   hw/s390x/s390-pci-inst.h |  2 +-
> > >   3 files changed, 40 insertions(+), 25 deletions(-)
> >
> > > @@ -700,8 +685,33 @@ int pcistb_service_call(S390CPU *cpu, uint8_t 
> > > r1, uint8_t r3, uint64_t gaddr,
> > >           break;
> > >       }
> > >   +    if (pcias > 5) {
> > > +        DPRINTF("pcistb invalid space\n");
> > > +        setcc(cpu, ZPCI_PCI_LS_ERR);
> > > +        s390_set_status_code(env, r1, ZPCI_PCI_ST_INVAL_AS);
> > > +        return 0;
> > > +    }
> > > +
> > > +    /* Verify the address, offset and length */
> > > +    /* offset must be a multiple of 8 */
> > > +    if (offset % 8) {
> > > +        goto addressing_error;
> > > +    }
> > > +    /* Length must be greater than 8, a multiple of 8, not greater 
> > > maxstbl */
> >
> > "not greater than maxstlb"
>
> Better I know but greater that 80 characters, this is why I preferred 
> broken English.
> What do I do ? break the line or English ?
less than?
> > > +    if ((len <= 8) || (len % 8) || (len > pbdev->maxstbl)) {
> > > +        goto addressing_error;
> > > +    }
> > > +    /* Do not cross a 4K-byte boundary */
> > > +    if (((offset & 0xfff) + len) > 0x1000) {
> > > +        goto addressing_error;
> > > +    }
> > > +    /* Guest address must be double word aligned */
> > > +    if (gaddr & 0x07UL) {
> > > +        goto addressing_error;
> > > +    }
> > > +
> > >       mr = pbdev->pdev->io_regions[pcias].memory;
> > > -    if (!memory_region_access_valid(mr, env->regs[r3], len, true)) {
> > > +    if (!memory_region_access_valid(mr, offset, len, true)) {
> > >           program_interrupt(env, PGM_OPERAND, 6);
> > >           return 0;
> > >       }
> >
> > Looks good.