Messages in this thread | | | Date | Mon, 21 Feb 2022 13:54:03 -0600 | Subject | Re: [PATCH v10 21/45] x86/mm: Add support to validate memory when changing C-bit | From | Brijesh Singh <> |
| |
On 2/21/22 11:41, Kirill A. Shutemov wrote: > On Wed, Feb 16, 2022 at 10:04:57AM -0600, Brijesh Singh wrote: >> @@ -287,6 +301,7 @@ struct x86_platform_ops { >> struct x86_legacy_features legacy; >> void (*set_legacy_features)(void); >> struct x86_hyper_runtime hyper; >> + struct x86_guest guest; >> }; > > I used 'cc' instead of 'guest'. 'guest' looks too generic.
I am fine with either of them.
> > Also, I'm not sure why not to use pointer to ops struct instead of stroing > them directly in x86_platform. Yes, it is consistent with 'hyper', but I > don't see it as a strong argument. > >> >> index b4072115c8ef..a55477a6e578 100644 >> --- a/arch/x86/mm/pat/set_memory.c >> +++ b/arch/x86/mm/pat/set_memory.c >> @@ -2012,8 +2012,15 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc) >> */ >> cpa_flush(&cpa, !this_cpu_has(X86_FEATURE_SME_COHERENT)); >> >> + /* Notify HV that we are about to set/clr encryption attribute. */ >> + x86_platform.guest.enc_status_change_prepare(addr, numpages, enc); >> + >> ret = __change_page_attr_set_clr(&cpa, 1); > > This doesn't cover difference in flushing requirements. Can we get it too? >
Yes, we can work to include that too.
>> >> + /* Notify HV that we have succesfully set/clr encryption attribute. */ >> + if (!ret) >> + x86_platform.guest.enc_status_change_finish(addr, numpages, enc); >> + > > Any particular reason you moved it above cpa_flush()? I don't think it > makes a difference for TDX, but still. >
It does not make any difference for the SNP as well. We can keep it where it was.
>> /* >> * After changing the encryption attribute, we need to flush TLBs again >> * in case any speculative TLB caching occurred (but no need to flush >> @@ -2023,12 +2030,6 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc) >> */ >> cpa_flush(&cpa, 0); >> >> - /* >> - * Notify hypervisor that a given memory range is mapped encrypted >> - * or decrypted. >> - */ >> - notify_range_enc_status_changed(addr, numpages, enc); >> - >> return ret; >> } >> >> -- >> 2.25.1 >> >
| |