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

[Cornelia Huck]

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"

> +    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.