Gustavo, Thanks for the review! It makes sense.
On Thu, Nov 06, 2025 at 12:44:23PM +0100, Gustavo Romero wrote: > Gabriel, given what Peter explained above, although the patch > is correct, the best to move forward here is to embedded this > patch into the additional series that implements the others > "subfeatures" and, as a last step, after the patches that > implement the features, you enable all the features in 'max'. Once I finish FEAT_MTE_STORE_ONLY I'll send a new series, which will include the changes from this series, the STORE_ONLY implementation, and also include patches for the updated documentation and tests for FEAT_MTE_TAGGED_FAR as you requested. As I continue to write patches for MTE4 features, I'll add them to that patchset and we can merge it all at once. Thanks, Gabriel On Thu, Nov 6, 2025 at 6:44 AM Gustavo Romero <[email protected]> wrote: > > Hi Peter, > > On 11/6/25 11:31, Peter Maydell wrote: > > On Wed, 5 Nov 2025 at 17:49, Gustavo Romero <[email protected]> > > wrote: > >> > >> Hi Gabriel, > >> > >> Thanks for your contribution. > >> > >> On 11/4/25 21:50, Gabriel Brookman wrote: > >>> FEAT_MTE_TAGGED_FAR is a feature required for MTE4. The feature > >>> guarantees that the full address (including tag bits) is reported after > >>> a SEGV_MTESERR, and advertises itself in the ID_AA64PFR2_EL1 system > >>> register. QEMU was already reporting the full address, so this commit > >>> simply advertises the feature by setting that register, and unsets the > >>> register if MTE is disabled. > >>> > >>> Signed-off-by: Gabriel Brookman <[email protected]> > >>> --- > >>> This patch is the first step toward implementing ARM's Enhanced Memory > >>> Tagging Extension (MTE4). MTE4 guarantees the presence of several > >>> subfeatures: FEAT_MTE_CANONICAL_TAGS, FEAT_MTE_TAGGED_FAR, > >>> FEAT_MTE_STORE_ONLY, FEAT_MTE_NO_ADDRESS_TAGS, and FEAT_MTE_PERM, > >>> none of which are currently implemented in QEMU. > >>> > >>> According to the ARM ARM, the presence of any of these features (except > >>> FEAT_MTE_PERM) implies the presence of all the others. For simplicity > >>> and ease of review, I plan to introduce them one at a time. This first > >>> patch focuses on FEAT_MTE_TAGGED_FAR. > >> > >> I think it's ok to add these "subfeatures" separately. > > > > We can add the implementation of the subfeatures separately, > > but we should not enable them in 'max' until they're all > > present. > > ah, true. I forget that when we do that we enable them in 'max' > as a last step. > > > > (We don't always adhere strictly to the architecture's > > "feature X implies Y exists" rules, > > Thanks for confirming it ;) > > > > but in this case > > because they're really a tightly linked bundle that add > > up to "MTE4" I think that presnting only a subset to guests > > is likely to result in them not behaving correctly.) > > Gabriel, given what Peter explained above, although the patch > is correct, the best to move forward here is to embedded this > patch into the additional series that implements the others > "subfeatures" and, as a last step, after the patches that > implement the features, you enable all the features in 'max'. > > > Cheers, > Gustavo
