Re: [PATCH v3 2/2] PCI: rcar: Return all Fs from read which triggered an exception

[Arnd Bergmann <arnd@arndb.de>]

On Sat, Jan 22, 2022 at 11:15 PM <marek.vasut@gmail.com> wrote:
>
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> In case the controller is transitioning to L1 in rcar_pcie_config_access(),
> any read/write access to PCIECDR triggers asynchronous external abort. This
> is because the transition to L1 link state must be manually finished by the
> driver. The PCIe IP can transition back from L1 state to L0 on its own.
>
> The current asynchronous external abort hook implementation restarts
> the instruction which finally triggered the fault, which can be a
> different instruction than the read/write instruction which started
> the faulting access. Usually the instruction which finally triggers
> the fault is one which has some data dependency on the result of the
> read/write. In case of read, the read value after fixup is undefined,
> while a read value of faulting read should be all Fs.
>
> It is possible to enforce the fault using 'isb' instruction placed
> right after the read/write instruction which started the faulting
> access. Add custom register accessors which perform the read/write
> followed immediately by 'isb'.
>
> This way, the fault always happens on the 'isb' and in case of read,
> which is located one instruction before the 'isb', it is now possible
> to fix up the return value of the read in the asynchronous external
> abort hook and make that read return all Fs.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Krzysztof Wilczyński <kw@linux.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: linux-renesas-soc@vger.kernel.org

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

provided once concern gets resolved:

> +#ifdef CONFIG_ARM
> +#define __rcar_pci_rw_reg_workaround(instr)                            \
> +               "1:     " instr " %1, [%2]\n"                           \
> +               "2:     isb\n"                                          \
> +               "3:     .pushsection .text.fixup,\"ax\"\n"              \
> +               "       .align  2\n"                                    \
> +               "4:     mov     %0, #" __stringify(PCIBIOS_SET_FAILED) "\n" \
> +               "       b       3b\n"                                   \
> +               "       .popsection\n"                                  \
> +               "       .pushsection __ex_table,\"a\"\n"                \
> +               "       .align  3\n"                                    \
> +               "       .long   1b, 4b\n"                               \
> +               "       .long   1b, 4b\n"                               \
> +               "       .popsection\n"
> +#endif

You list the fixup for the ldr/str instruction here twice, (.long 1b,4b), but
no fixup for the isb instruction (.long 2b, 4b). Your description says that
the fault happens on the isb, not the ldr, so I don't understand what is
going on here.

       Arnd