Re: [Bug Report] Multiple S390x KUNIT clk failures

Stephen Boyd
2024-05-29T19:39:36Z

Quoting msalter@redhat.com (2024-05-28 15:53:48)
> On Tue, 2024-05-28 at 16:49 -0400, Audra Mitchell wrote:
> >
> > I spent some time last week or so working on debugging these failures and I
> > believe I have found the problem. I reached out to Malk Salter for advice on
> > the best way to move forward with a fix on Friday the 17th, but he was on
> > PTO for the last week. I was waiting for his reply before I replied to this
> > thread.
> >
> > Also as a side note, I also ran into the same issue as Stephen with running
> > the kunit tests on s390 QEMU. I did not pursue resolving that issue and
> > instead just compiled the test as a module.
> >
> > For clarity, this is what I sent to Mark and were I believe the failure is
> > occurring:
> >
> > The tests create a pretend clk-gate and use a "fake_reg" to emulate
> > the expected behavior of the clk_gate->reg. I added some debug
> > statements to the driver and noticed that the reg changes after
> > initialization to -1. I also noticed that we call this to read the
> > data in the clk-gate->reg:
> >
> > static inline u32 clk_gate_readl(struct clk_gate *gate)
> > {
> >         if (gate->flags & CLK_GATE_BIG_ENDIAN)
> >                 return ioread32be(gate->reg);
> >
> >         return readl(gate->reg);
> > }
> >
> > However, it does not look like ioread32be is defined for s390, so
>
> It is defined. arch/s390/include/asm/io.h defines:
>
> #define __raw_readl zpci_read_u32
>
> and then includes include/asm-generic/io.h which has:
>
> static inline u32 readl(const volatile void __iomem *addr)
> {
> u32 val;
>
> log_read_mmio(32, addr, _THIS_IP_, _RET_IP_);
> __io_br();
> val = __le32_to_cpu((__le32 __force)__raw_readl(addr));
> __io_ar(val);
> log_post_read_mmio(val, 32, addr, _THIS_IP_, _RET_IP_);
> return val;
> }
> ...
> static inline u32 ioread32be(const volatile void __iomem *addr)
> {
> return swab32(readl(addr));
> }
>
> which should do the right thing (s390 being BE and readl() is for 32-bit LE reads).
>
> But I don't know the s390 compiler or ISA, so I'm not sure where the zpci_load
> is coming from.
>

So the problem is that the zpci_read_u32() fails and returns -1?

This test isn't the best because it uses fakes iomem and architectures
may not like that. We really need to implement something in KUnit core
to allocate a fake iomem region and then plumb that through all the
architectures so that the iomem functions like readl, writel, etc. go a
different direction when the pointer is for the fake region.

Probably the best thing to do in the short term here is to prevent this
test from running on S390 via Kconfig.