On Tue, 21 Nov 2017 11:38:45 +0100 Cornelia Huck <coh...@redhat.com> wrote:
> On Thu, 16 Nov 2017 18:51:50 +0100 > Pierre Morel <pmo...@linux.vnet.ibm.com> wrote: > > @@ -493,9 +494,13 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, > > uint8_t r2) > > break; > > } > > > > - data = env->regs[r1]; > > - if (pcias < 6) { > > - if ((8 - (offset & 0x7)) < len) { > > + /* Test that the pcias is valid and do the appropriates operations */ > > + switch (pcias) { > > + case 0 ... 5: > > Make this > case 0...5: > ? ...only that it confuses the compiler when using numbers. We can either keep the slightly ugly version, or introduce #defines. ZPCI_PCIAS_IOREGION_{MIN,MAX} (and maybe ZPCI_PCIAS_CONFIG for 15)? > > > + /* Check length: > > + * A length of 0 is invalid and length should not cross a double > > word > > + */ > > + if (!len || (len > (8 - (offset & 0x7)))) { > > program_interrupt(env, PGM_OPERAND, 4); > > return 0; > > } > > @@ -513,21 +518,21 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, > > uint8_t r2) > > program_interrupt(env, PGM_OPERAND, 4); > > return 0; > > } > > - } else if (pcias == 15) { > > - if ((4 - (offset & 0x3)) < len) { > > - program_interrupt(env, PGM_OPERAND, 4); > > - return 0; > > - } > > - > > - if (zpci_endian_swap(&data, len)) { > > + break; > > + case 15: > > + /* ZPCI uses the pseudo BAR number 15 as configuration space */ > > + /* possible access lengths are 1,2,4 and must not cross a word */ > > + if (!len || (len > (4 - (offset & 0x3))) || len == 3) { > > program_interrupt(env, PGM_OPERAND, 4); > > return 0; > > } > > - > > + /* len = 1,2,4 so we do not need to test */ > > + zpci_endian_swap(&data, len); > > pci_host_config_write_common(pbdev->pdev, offset, > > pci_config_size(pbdev->pdev), > > data, len); > > - } else { > > + break; > > + default: > > DPRINTF("pcistg invalid space\n"); > > setcc(cpu, ZPCI_PCI_LS_ERR); > > s390_set_status_code(env, r2, ZPCI_PCI_ST_INVAL_AS); > > Other than the nits, looks good.