On 3/10/2022 8:46 PM, Dave Hansen wrote: > On 3/10/22 03:15, Bharata B Rao wrote:> >> This patchset builds on that prctl() extension and adds support >> for AMD UAI. AMD implementation is kept separate as equivalent >> Intel LAM implementation is likely to be different due to different >> bit positions and tag width. > > Please don't keep the implementations separate. > > We'll have one x86 implementation of address bit masking. Both the > Intel and AMD implementations will feed into a shared implementation. > Something _like_ the cc_set_mask() interface where both implementations > do their detection and then call into common code to say how many bits > are being ignored. The difference isn't limited to the number of ignored bits, see below... > > A good litmus test for this is how many vendor-specific checks there are > in common code. If there are a lot of them, it's not a good sign for > the design. That is generally true but considering the below differences, I felt it was much cleaner to go for separate implementations for enablement and untagging while still building on the common prctl() extension in the RFC version. 1. While Intel LAM provides two LAM widths (15 and 6 bits wide), AMD UAI has a fixed tag width of 7 bits. This results in following differences in the implementation: - Two threadinfo flags (TIF_LAM_57 and TIF_LAM_48) in Intel LAM vs single flag TIF_TAGGED_ADDR(like in ARM64) in AMD UAI. - Untagging needs to be aware of 2 widths in Intel LAM as against a single width in AMD UAI. - Requirement of making LAM_U48 and mappings above 47bits mutually exclusive is required for Intel LAM only. 2. The enablement bit is part of CR3 in Intel LAM which brings in additional complexity of updating the CR3 with right LAM mode on every MM switch and associated tlbstate changes. In AMD UAI, enablement bit is part of EFER MSR and it is a single time enablement with no MM switch time changes. However, let me take a detailed look once again and explore further code sharing possibilities. Regards, Bharata.