On Tue, Jul 25, 2023 at 11:01:16AM +0100, David Woodhouse wrote:
> From: David Woodhouse <d...@amazon.co.uk>
>
> A generic X86IOMMUClass->int_remap function should not return VT-d
> specific values; fix it to return 0 if the interrupt was successfully
> translated or -EINVAL if not.
>
> The VTD_FR_IR_xxx values are supposed to be used to actually raise
> faults through the fault reporting mechanism, so do that instead for
> the case where the IRQ is actually being injected.
>
> There is more work to be done here, as pretranslations for the KVM IRQ
> routing table can't fault; an untranslatable IRQ should be handled in
> userspace and the fault raised only when the IRQ actually happens (if
> indeed the IRTE is still not valid at that time). But we can work on
> that later; we can at least raise faults for the direct case.
>
> Signed-off-by: David Woodhouse <d...@amazon.co.uk>
Mostly good to me, only still a few trivial comments..
[...]
> @@ -3320,17 +3342,33 @@ static int vtd_irte_get(IntelIOMMUState *iommu,
> uint16_t index,
> entry, sizeof(*entry), MEMTXATTRS_UNSPECIFIED)) {
> error_report_once("%s: read failed: ind=0x%x addr=0x%" PRIx64,
> __func__, index, addr);
> - return -VTD_FR_IR_ROOT_INVAL;
> + if (do_fault) {
> + vtd_report_ir_fault(iommu, sid, VTD_FR_IR_ROOT_INVAL, index);
> + }
> + return false;
> }
>
> trace_vtd_ir_irte_get(index, le64_to_cpu(entry->data[1]),
> le64_to_cpu(entry->data[0]));
>
> + /*
> + * The remaining potential fault conditions are "qualified" by the
> + * Fault Processing Disable bit in the IRTE. Even "not present".
> + * So just clear the do_fault flag if PFD is set, which will
> + * prevent faults being raised.
> + */
> + if (entry->irte.fault_disable) {
> + do_fault = false;
Wrong indent on these lines..
> + }
> +
> if (!entry->irte.present) {
> error_report_once("%s: detected non-present IRTE "
> "(index=%u, high=0x%" PRIx64 ", low=0x%" PRIx64
> ")",
> __func__, index, le64_to_cpu(entry->data[1]),
> le64_to_cpu(entry->data[0]));
> + if (do_fault) {
> + vtd_report_ir_fault(iommu, sid, VTD_FR_IR_ENTRY_P, index);
> + }
> return -VTD_FR_IR_ENTRY_P;
Forgot to change retval?
> }
Thanks,
--
Peter Xu
