On 01/07/2019 07:07, David Gibson wrote: > On Fri, Jun 21, 2019 at 05:05:45PM +0200, Cédric Le Goater wrote: >> On 21/06/2019 16:52, Greg Kurz wrote: >>> As indicated in the function header, this "hcall is only supported for >>> LISNs that have the ESB hcall flag set to 1 when returned from hcall() >>> H_INT_GET_SOURCE_INFO". We only set that flag for LSIs actually. >>> >>> Check that in h_int_esb(). >> >> Indeed. H_INT_ESB should work on any IRQ, but I think it's better >> to check that the HCALL is only used with the IRQ requiring it. > > I'm not so convinced. It seems to me that specifically limiting this > to certain things limits our options if we ever need future > workarounds for problems with ESB mapping. > > In addition using H_INT_ESB for non-LSIs could be useful for minimal > guests (e.g. kvm-unit-tests) where mapping memory might be awkward.
Ah yes. This is true. There is no real reason for enforcing this restriction in QEMU as H_INT_ESB should work on any irq. We can keep it that way I guess. It would be a small deviation from PAPR. Thanks, C. > So, I'm not really seeing a compelling reason to apply this. > >> >>> Signed-off-by: Greg Kurz <gr...@kaod.org> >> >> Reviewed-by: Cédric Le Goater <c...@kaod.org> >> >> Thanks, >> >> C. >> >>> --- >>> hw/intc/spapr_xive.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >>> index 58c2e5d890bd..01dd47ad5b02 100644 >>> --- a/hw/intc/spapr_xive.c >>> +++ b/hw/intc/spapr_xive.c >>> @@ -1408,6 +1408,12 @@ static target_ulong h_int_esb(PowerPCCPU *cpu, >>> return H_P2; >>> } >>> >>> + if (!xive_source_irq_is_lsi(xsrc, lisn)) { >>> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: LISN " TARGET_FMT_lx "isn't >>> LSI\n", >>> + lisn); >>> + return H_P2; >>> + } >>> + >>> if (offset > (1ull << xsrc->esb_shift)) { >>> return H_P3; >>> } >>> >> >