On Thu, Jan 03, 2019 at 06:45:25PM +0100, Cédric Le Goater wrote:
> On 1/3/19 5:33 AM, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 06:57:41AM +0100, Cédric Le Goater wrote:
> >> commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
> >> uninitialized") introduced an extra check on the ICS offset which is
> >> not strictly necessary.
> >
> > The commit message for that suggests it was added to make pnv easier.
> > I take it you no longer need this for the current or expected pnv
> > code?
>
> This is really just a runtime check which makes sure that the
> ICSState offset is initialized by the FW before being used.
>
> The problem is more global. It comes from the fact the ICSState
> offset in QEMU and the XICS offset in KVM is hardcoded to 0x1000.
>
> Thinking aloud :
>
> May be we should default the offset to 0 and introduce a service
> to set the value in QEMU and KVM.
>
> The 'dual' interrupt mode (using the qirq array at the machine level)
> needs the IRQ number space of the XIVE and XICS interrupt mode to
> be in sync, that is to start a 0x0, even if the lower 4K are ignored
> in XICS.
>
> So the 'dual' mode tunes the offset to 0 once the ICSState is created
> and ignores the lower 4K when initializing the KVM XICS device.
Hm, ok.
I've applied patches 5..8.
>
> C.
>
> >> Revert the change to be able to map the XICS IRQ number space on the
> >> XIVE IRQ number space.
> >
> >
> >
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >> include/hw/ppc/xics.h | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> >> index 7668c381a887..07508cbd217e 100644
> >> --- a/include/hw/ppc/xics.h
> >> +++ b/include/hw/ppc/xics.h
> >> @@ -139,8 +139,7 @@ struct ICSState {
> >>
> >> static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
> >> {
> >> - return (ics->offset != 0) && (nr >= ics->offset)
> >> - && (nr < (ics->offset + ics->nr_irqs));
> >> + return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
> >> }
> >>
> >> struct ICSIRQState {
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson