Re: [PATCH v3 2/4] xen: make VMAP only support in MMU system

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 16/08/2024 10:28, Michal Orzel wrote:
> Hi Ayan,
Hi Michal,
> On 14/08/2024 14:33, Ayan Kumar Halder wrote:
> > Hi Jan,
> >
> > On 14/08/2024 12:35, Jan Beulich wrote:
> > > On 14.08.2024 12:55, Ayan Kumar Halder wrote:
> > > > Hi Jan,
> > > >
> > > > On 14/08/2024 07:37, Jan Beulich wrote:
> > > > > On 13.08.2024 19:13, Ayan Kumar Halder wrote:
> > > > > > From: Penny Zheng <penny.zheng@xxxxxxx>
> > > > > >
> > > > > > Introduced CONFIG_VMAP which is selected by the architectures that use
> > > > > > MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled.
> > > > > >
> > > > > > VMAP is widely used in ALTERNATIVE feature to remap a range of memory
> > > > > > with new memory attributes. Since this is highly dependent on virtual
> > > > > > address translation, we choose to fold VMAP in MMU system.
> > > > > >
> > > > > > In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it
> > > > > > only support in MMU system on ARM architecture. And ALTERNATIVE now
> > > > > > depends on VMAP.
> > > > > >
> > > > > > HARDEN_BRANCH_PREDICTOR is now gated on HAS_VMAP as speculative
> > > > > > attacks are not possible on non MMU based systems (ie Cortex-R52, R82).
> > > > > > See 
> > > > > > https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability.
> > > > > While I'm not an Arm expert and hence I'm likely missing aspects, I question
> > > > > the one (Spectre-BHB) vulnerability there to be sufficient to draw a
> > > > > conclusion towards the usefulness of branch hardening. I would advise
> > > > > against encoding such a connection in the Kconfig dependencies.
> > > > AFAIU, to address 'Spectre' like vulnerabilities 'branch hardening' was
> > > > added.
> > > >
> > > > See https://lore.kernel.org/all/E1fNadD-0000fz-9r@xxxxxxxxxxxxxxxxxxxxxx/
> > > >
> > > > And from
> > > > https://lists.linaro.org/archives/list/linux-stable-mirror@xxxxxxxxxxxxxxxx/message/F4MGL4WT2R7T54NLRDGKFRQNSXF3DZGD/
> > > >
> > > > Spectre is valid on MMU based systems.
> > > Since then various other issues / flavors were found. I've been focusing
> > > on the x86 side of things, but I'd be very surprised if some didn't
> > > affect other architectures as well.
> > We are talking about Arm here as "HARDEN_BRANCH_PREDICTOR" is specific
> > to Arm.
> >
> > https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability
> > covers all the flavours and it does not include Cortex-R82 or R52.
> >
> > It says the following :-
> >
> > "Cortex-R cores typically use a closed software stack. In those
> > environments, applications or processes are strictly controlled, and
> > therefore not exploitable"
> >
> > > Plus branch hardening can be a pre-
> > > cautionary measure, too, I think.
> > The first two Arm non MMU cores that we wish to support in the
> > forthcoming series is Cortex-R82 and R52.
> >
> > As seen in https://developer.arm.com/documentation/ka005109/latest/, it
> > explicitly states the following about R82
> >
> > The Cortex-R82 implements the faulting feature (FEAT_FPAC) but is not
> > vulnerable. The Cortex-R82 behaves differently than vulnerable A-class
> > CPUs when speculatively executing past an instruction that authenticates
> > PAC, and that behavior does not allow the attack software to create the
> > "oracle".
> >
> > We can re-enable branch hardening if we know there is a valid non MMU
> > Arm core which is vulnerable.
> >
> > Let me know if you are ok with the rationale.
> I'm ok with your rationale.
>
> I have one question for this patch. Why can't we use CONFIG_HAS_VMAP to 
> conditionally compile vmap.c, like:
> obj-$(CONFIG_HAS_VMAP) += vmap.o
> and get rid of VMAP_VIRT_START guard on an entire file?
> With this config in place, it seems strange to use VMAP_VIRT_START as a guard.
I am fine with your suggestion.

However, this implies HAS_VMAP needs to be selected by "config PPC" as 
well (as it defines VMAP_VIRT_START in xen/arch/ppc/include/asm/config.h ).
This is fine as I could see PPC uses MMU as well.

- Ayan