On Fri, Dec 13, 2024 at 02:34:57PM -0800, Yang Shi wrote:

On 12/13/24 8:17 AM, Mikołaj Lenczewski wrote:

quoted

quoted

quoted

+static int do_conflict_abort(unsigned long far, unsigned long esr,
+			     struct pt_regs *regs)
+{
+	if (!system_supports_bbml2())
+		return do_bad(far, esr, regs);
+
+	/* if we receive a TLB conflict abort, we know that there are multiple
+	 * TLB entries that translate the same address range. the minimum set
+	 * of invalidations to clear these entries is implementation defined.
+	 * the maximum set is defined as either tlbi(vmalls12e1) or tlbi(alle1).
+	 *
+	 * if el2 is enabled and stage 2 translation enabled, this may be
+	 * raised as a stage 2 abort. if el2 is enabled but stage 2 translation
+	 * disabled, or if el2 is disabled, it will be raised as a stage 1
+	 * abort.
+	 *
+	 * local_flush_tlb_all() does a tlbi(vmalle1), which is enough to
+	 * handle a stage 1 abort.
+	 */
+
+	local_flush_tlb_all();
+
+	return 0;
+}

Can we actually guarantee that we make it this far without taking another
abort? Given that I'm yet to see one of these things in the wild, I'm
fairly opposed to pretending that we can handle them. We'd be much better
off only violating BBM on CPUs that are known to handle the conflict
gracefully. Judging by your later patch, this is practically keyed off
the MIDR _anyway_...

Will

Hi Mikołaj,

quoted

Thanks for reviewing. Apologies for the delay in responding, and for
spam (replied instead of group-replied).

There should not be an option to take another fault while performing the
handler, as long as the mappings covering the fault handler table or any
code in this path are not screwed with. This is discussed further in the
resent patch series [1].

Will lead me to this thread when we discussed about my series
(https://lore.kernel.org/lkml/20241211223034.GA17836@willie-the-truck/#t (local)).
My series tried to use large mapping for kernel linear mapping when
rodata=full. The common part of the both series is BBM lv2 support. My
series depends on BBM lv2 to change page table block size when changing
permission for kernel linear mapping.

Handling TLB conflict should be ok for your usecase since the conflict
should just happen in user address space. But it may be not fine for my
usecase. At least the TLB conflict handler needs to depend on
CONFIG_VMAP_STACK otherwise the kernel stack pointer points to kernel linear
address space. I'm not quite sure whether there is any other corner case
which can trigger recursive abort in the path or not, maybe bad stack
handler? So Will suggested MIDR based lookup. IIUC, he suggested just enable
BBM lv2 support on the CPUs which can handle TLB conflict gracefully. This
should make our lives easier. But the downside is maybe fewer CPUs can
actually advertise BBM lv2 support.

Since most of them seem to be broken anyway, I still think this (having
an allowlist and not handling the conflict abort in the kernel) is the
right approach.

Will