[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/7] s390x/pci: rework PCI STORE BLOCK
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH 4/7] s390x/pci: rework PCI STORE BLOCK |
Date: |
Mon, 13 Nov 2017 16:23:28 +0100 |
On Tue, 7 Nov 2017 18:24:36 +0100
Pierre Morel <address@hidden> wrote:
> Enhance the fault detection.
>
> 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(-)
>
> @@ -662,22 +664,10 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1,
> uint8_t r3, uint64_t gaddr,
> fh = env->regs[r1] >> 32;
> pcias = (env->regs[r1] >> 16) & 0xf;
> len = env->regs[r1] & 0xff;
> + offset = env->regs[r3];
>
> - 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;
> - }
> -
> - switch (len) {
> - case 16:
> - case 32:
> - case 64:
> - case 128:
> - break;
> - default:
> - program_interrupt(env, PGM_SPECIFICATION, 6);
> + if (!(fh & FH_MASK_ENABLE)) {
> + setcc(cpu, ZPCI_PCI_LS_INVAL_HANDLE);
So this means you move checking for the device before checking for the
parameters or the as.
> return 0;
> }
>
> @@ -689,12 +679,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1,
> uint8_t r3, uint64_t gaddr,
> }
>
> switch (pbdev->state) {
> - case ZPCI_FS_RESERVED:
> - case ZPCI_FS_STANDBY:
> - case ZPCI_FS_DISABLED:
> case ZPCI_FS_PERMANENT_ERROR:
> - setcc(cpu, ZPCI_PCI_LS_INVAL_HANDLE);
> - return 0;
> case ZPCI_FS_ERROR:
> setcc(cpu, ZPCI_PCI_LS_ERR);
> s390_set_status_code(env, r1, ZPCI_PCI_ST_BLOCKED);
> @@ -703,8 +688,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, lower than maxstbl */
> + 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;
> + }
So the checks here are only evaluated if the instruction actually pokes
at a valid region?
> +
> 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;
> }
> @@ -714,9 +724,9 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t
> r3, uint64_t gaddr,
> }
>
> for (i = 0; i < len / 8; i++) {
> - result = memory_region_dispatch_write(mr, env->regs[r3] + i * 8,
> - ldq_p(buffer + i * 8), 8,
> - MEMTXATTRS_UNSPECIFIED);
> + result = memory_region_dispatch_write(mr, offset + i * 8,
> + ldq_p(buffer + i * 8), 8,
> + MEMTXATTRS_UNSPECIFIED);
> if (result != MEMTX_OK) {
> program_interrupt(env, PGM_OPERAND, 6);
> return 0;
> @@ -725,6 +735,10 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1,
> uint8_t r3, uint64_t gaddr,
>
> setcc(cpu, ZPCI_PCI_LS_OK);
> return 0;
> +
> +addressing_error:
> + program_interrupt(env, PGM_SPECIFICATION, 6);
> + return 0;
> }
This seems more readable; I can't verify whether it is actually correct
without access to the architecture, though :(
- Re: [Qemu-devel] [PATCH 1/7] s390x/pci: factor out endianess conversion, (continued)
[Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Pierre Morel, 2017/11/07
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Cornelia Huck, 2017/11/09
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Yi Min Zhao, 2017/11/10
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Cornelia Huck, 2017/11/10
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Pierre Morel, 2017/11/13
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Pierre Morel, 2017/11/13
- Re: [Qemu-devel] [PATCH 6/7] s390x/pci: move the memory region write from pcistg, Cornelia Huck, 2017/11/13