On 261002 19:43, Konstantin Shkolnyy wrote:
On 261002 13:28, Matthew Rosato wrote:
-static int reg_ioat(CPUS390XState *env, S390PCIBusDevice *pbdev,
ZpciFib fib,
- uintptr_t ra)
+bool s390_pci_ioat_validate(S390PCIBusDevice *pbdev, uint64_t pba,
+ uint64_t pal, uint64_t g_iota, Error
**errp)
{
- uint64_t pba = ldq_be_p(&fib.pba);
- uint64_t pal = ldq_be_p(&fib.pal);
- uint64_t g_iota = ldq_be_p(&fib.iota);
uint8_t dt = (g_iota >> 2) & 0x7;
bool t = s390_pci_is_translation_enabled(g_iota);
- pba &= ~0xfff;
- pal |= 0xfff;
if (pba > pal || pba < pbdev->zpci_fn.sdma || pal > pbdev-
>zpci_fn.edma) {
- s390_program_interrupt(env, PGM_OPERAND, ra);
- return -EINVAL;
+ return false;
This will now violate include/qapi/error.h because you're returning
false but not setting errp:
* - On success, the function should not touch *errp. On failure, it
* should set a new error, e.g. with error_setg(errp, ...), or
* propagate an existing one, e.g. with error_propagate(errp, ...).
A simple solution would be to add a new message for this case.
But the original reg_ioat() also didn't print any message in this error
branch. I had to assume it was intentional, and preserved the behavior.
If you believe a new message should be introduced, I can insert a commit
for that - what would be the explanation?
Other code sites reporting PGM_OPERAND also don't generate error
messages - it appears, the current policy is not to do that in the cases
when the guest is at fault (gave invalid parameters). The case at hand
falls in that category.
[...]
+
+static int reg_ioat(CPUS390XState *env, S390PCIBusDevice *pbdev,
ZpciFib fib,
+ uintptr_t ra)
+{
+ Error *err = NULL;
+ uint64_t pba = ldq_be_p(&fib.pba);
+ uint64_t pal = ldq_be_p(&fib.pal);
+ uint64_t g_iota = ldq_be_p(&fib.iota);
+ bool t = s390_pci_is_translation_enabled(g_iota);
+
+ pba &= ~0xfff;
+ pal |= 0xfff;
+
+ if (!s390_pci_ioat_validate(pbdev, pba, pal, g_iota, &err)) {
+ if (err) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s\n",
error_get_pretty(err));
+ error_free(err);
+ }
With that change, you won't need the if (err) check anymore.
Thanks,
Matt
