On 27-Jun-16 15:49, Jan Kiszka wrote: > On 2016-06-27 15:21, Antonios Motakis wrote: >> >> >> On 27-Jun-16 13:11, Jan Kiszka wrote: >>> On 2016-06-25 08:30, Jan Kiszka wrote: >>>> On 2016-06-17 21:13, [email protected] wrote: >>>>> From: Antonios Motakis <[email protected]> >>>>> >>>>> The AMD Seattle board features SPI ids that are larger than 64, >>>>> which we do not support properly. This workaround allows us to >>>>> demonstrate working cells on this target, until we have a proper fix. >>>>> >>>>> This implies that only specific setups will be used on the >>>>> AMD Seattle; the IRQs for the uart and the second xgmac are being >>>>> passed to the cells. >>>>> >>>>> Signed-off-by: Antonios Motakis <[email protected]> >>>>> --- >>>>> hypervisor/arch/arm/irqchip.c | 17 +++++++++++++++-- >>>>> 1 file changed, 15 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/hypervisor/arch/arm/irqchip.c b/hypervisor/arch/arm/irqchip.c >>>>> index d14de0a..7b9b429 100644 >>>>> --- a/hypervisor/arch/arm/irqchip.c >>>>> +++ b/hypervisor/arch/arm/irqchip.c >>>>> @@ -40,9 +40,22 @@ bool spi_in_cell(struct cell *cell, unsigned int spi) >>>>> /* FIXME: Change the configuration to a bitmask range */ >>>>> u32 spi_mask; >>>>> >>>>> - if (spi >= 64) >>>>> + if (spi >= 64) { >>>>> +#ifdef CONFIG_MACH_AMD_SEATTLE >>>>> + /* uart irq workaround */ >>>>> + if (spi == 328) >>>>> + return (cell != &root_cell); >>>>> + >>>>> + /* xgmac1 irq workaround for the very brave. >>>>> + * Uncommenting this may make the root cell unstable. >>>>> + if ((spi == 322) || (spi ==324) || >>>>> + ((spi >= 341) && (spi <= 345))) { >>>>> + >>>>> + return (cell != &root_cell); >>>>> + }*/ >>>> >>>> Ah, this was eating my interrupts! Uncommented, and now it works again. >>>> >>>> I was always unbinding the xgmac1 before creating the non-root Linux >>>> cell, plus that interface was down all the time while Jailhouse was >>>> enabled. Therefore no instability here. >>>> >>>> Anyway, this is nasty, and we need to address this properly before >>>> merging. In fact, I may have a need for proper SPI assignment on ARM >>>> soon as well. Brings back the proposal I still like most: >>>> >>>>> - use multiple irqchip entries per physical chip to add more bitmap >>>>> capacity if needed >>>>> >>>>> The latter could be modelled as >>>>> >>>>> struct jailhouse_irqchip { >>>>> __u64 address; >>>>> __u64 id; >>>>> __u64 pin_base; >>>>> __u64 pin_bitmap[BITMAP_SIZE]; >>>>> } >>>>> >>>> >>>> Let me play with that... >>>> >>> >>> Done, results in next and a rebased version of your patches in >>> wip/arm64. Seems to work, but the GIC v3 is still totally untested. >>> Feedback welcome! >>> >>> Will send the base patches soon, then look into the ARM64 series in more >>> details. >> >> Looks good! (yeah, I ate your interrupts, sorry :) >> >> Actually, I also unbind the second xgmac from my side. However, I would have >> nasty stuff happen when first loading a Linux inmate, and then destroying it >> without disabling Jailhouse. Eventually CPU6 (which was CPU0 for the inmate >> previously), would lock up. In no other scenario would I see the same lock >> up. >> >> The most plausible explanation I have, is that the instability was not only >> due to the IRQ handling, but also because platform devices, such as the NIC >> here, don't have a standard reset method. This means that the NIC is not in >> a reset state after destroying the inmate. The combination of a non-reset >> NIC, with the sub-optimal IRQ handling, probably caused the lock ups. >> >> It will be interesting to see now that the IRQ mapping to the cells has been >> fixed, whether this scenario is still a problem. > > OK, keep me informed. I will try to finish the review based on wip/arm64 > soon. > > BTW, one background question: How do you work around the missing SMMU > support for the non-root cell when the xgmac does DMA? I see in the cell > config that only half of the memory is 1:1 mapped. Is the lower half > never used as DMA target by Linux?
Actually, what happens is we are being super wasteful: only the second region is used by Linux at all (see the provided device tree). We still need a zero-mapped region for the loader, plus we also put the DTB there. But looking at this again, we definitely don't need as much... Tony -- Antonios Motakis Virtualization Engineer Huawei Technologies Duesseldorf GmbH European Research Center Riesstrasse 25, 80992 München -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
