Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 9/6/18 1:50 PM, Brijesh Singh wrote: ... >> >> #define HVC_DECRYPTED_ARRAY_SIZE  \ >> PAGE_ALIGN((NR_CPUS - HVC_BOOT_ARRAY_SIZE) * \ >>    sizeof(struct pvclock_vsyscall_time_info)) >> > Since the hv_clock_aux array will have NR_CPUS elements hence the following definition is

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 9/6/18 1:50 PM, Brijesh Singh wrote: ... >> >> #define HVC_DECRYPTED_ARRAY_SIZE  \ >> PAGE_ALIGN((NR_CPUS - HVC_BOOT_ARRAY_SIZE) * \ >>    sizeof(struct pvclock_vsyscall_time_info)) >> > Since the hv_clock_aux array will have NR_CPUS elements hence the following definition is

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 03:39 PM, Sean Christopherson wrote: On Thu, Sep 06, 2018 at 03:20:46PM -0500, Brijesh Singh wrote: On 09/06/2018 02:47 PM, Sean Christopherson wrote: ... Yes, the auxiliary array will dumped into the regular .bss when CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k,

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 03:39 PM, Sean Christopherson wrote: On Thu, Sep 06, 2018 at 03:20:46PM -0500, Brijesh Singh wrote: On 09/06/2018 02:47 PM, Sean Christopherson wrote: ... Yes, the auxiliary array will dumped into the regular .bss when CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k,

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 03:20:46PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 02:47 PM, Sean Christopherson wrote: > ... > > >> > >>Yes, the auxiliary array will dumped into the regular .bss when > >>CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k, I am not > >>sure if its worth

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 03:20:46PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 02:47 PM, Sean Christopherson wrote: > ... > > >> > >>Yes, the auxiliary array will dumped into the regular .bss when > >>CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k, I am not > >>sure if its worth

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 02:47 PM, Sean Christopherson wrote: ... Yes, the auxiliary array will dumped into the regular .bss when CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k, I am not sure if its worth complicating the code to save those extra memory. Most of the distro's have

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 02:47 PM, Sean Christopherson wrote: ... Yes, the auxiliary array will dumped into the regular .bss when CONFIG_AMD_MEM_ENCRYPT=n. Typically it will be few k, I am not sure if its worth complicating the code to save those extra memory. Most of the distro's have

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 02:24 PM, Brijesh Singh wrote: ... Again we have to consider the bare metal scenario while doing this. The aux array you proposed will be added in decrypted section only when CONFIG_AMD_MEM_ENCRYPT=y.  If CONFIG_AMD_MEM_ENCRYPT=n then nothng gets put in .data.decrypted

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 02:24 PM, Brijesh Singh wrote: ... Again we have to consider the bare metal scenario while doing this. The aux array you proposed will be added in decrypted section only when CONFIG_AMD_MEM_ENCRYPT=y.  If CONFIG_AMD_MEM_ENCRYPT=n then nothng gets put in .data.decrypted

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 02:24:32PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 01:47 PM, Sean Christopherson wrote: > >On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: > >> > >> > >>On 09/06/2018 09:18 AM, Sean Christopherson wrote: > >> > >> > > > >So are we going

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 02:24:32PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 01:47 PM, Sean Christopherson wrote: > >On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: > >> > >> > >>On 09/06/2018 09:18 AM, Sean Christopherson wrote: > >> > >> > > > >So are we going

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 01:47 PM, Sean Christopherson wrote: On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: On 09/06/2018 09:18 AM, Sean Christopherson wrote: So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 01:47 PM, Sean Christopherson wrote: On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: On 09/06/2018 09:18 AM, Sean Christopherson wrote: So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 11:45:03AM -0700, Sean Christopherson wrote: > Yep, though because the 4k chunk in __decrypted is @hv_clock_boot > that's used for cpus 0-127, __decrypted_XXX would effectively be: > >(((num_possible_cpus() * 32) / 4k) - 1) pages Ok, sounds like a nice compromise to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 11:45:03AM -0700, Sean Christopherson wrote: > Yep, though because the 4k chunk in __decrypted is @hv_clock_boot > that's used for cpus 0-127, __decrypted_XXX would effectively be: > >(((num_possible_cpus() * 32) / 4k) - 1) pages Ok, sounds like a nice compromise to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 09:07 AM, Sean Christopherson wrote: ... + +/* This should cover upto 512 VCPUS (first 64 are covered by hv_clock_boot[]). */ +#define HVC_DECRYPTED_ARRAY_SIZE \ + ((PAGE_SIZE * 7) / sizeof(struct pvclock_vsyscall_time_info)) I think we can define the size relative to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 09:07 AM, Sean Christopherson wrote: ... + +/* This should cover upto 512 VCPUS (first 64 are covered by hv_clock_boot[]). */ +#define HVC_DECRYPTED_ARRAY_SIZE \ + ((PAGE_SIZE * 7) / sizeof(struct pvclock_vsyscall_time_info)) I think we can define the size relative to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 09:18 AM, Sean Christopherson wrote: > > > >>> > >>>So are we going to be defining a decrypted section for every piece of > >>>machinery now? > >>> > >>>That's a bit too much in my book. > >>> > >>>Why can't

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 01:37:50PM -0500, Brijesh Singh wrote: > > > On 09/06/2018 09:18 AM, Sean Christopherson wrote: > > > >>> > >>>So are we going to be defining a decrypted section for every piece of > >>>machinery now? > >>> > >>>That's a bit too much in my book. > >>> > >>>Why can't

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 08:33:34PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: > > My thought was that we could simply define a second array for the SEV > > case to statically allocate for NR_CPUS since __decrypted has a big > > chunk of

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 08:33:34PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: > > My thought was that we could simply define a second array for the SEV > > case to statically allocate for NR_CPUS since __decrypted has a big > > chunk of

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 01:33 PM, Borislav Petkov wrote: On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: My thought was that we could simply define a second array for the SEV case to statically allocate for NR_CPUS since __decrypted has a big chunk of memory that would be ununsed

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 01:33 PM, Borislav Petkov wrote: On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: My thought was that we could simply define a second array for the SEV case to statically allocate for NR_CPUS since __decrypted has a big chunk of memory that would be ununsed

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 09:18 AM, Sean Christopherson wrote: So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my book. Why can't you simply free everything in .data..decrypted on !SVE guests? That would prevent adding __decrypted

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 09:18 AM, Sean Christopherson wrote: So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my book. Why can't you simply free everything in .data..decrypted on !SVE guests? That would prevent adding __decrypted

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: > My thought was that we could simply define a second array for the SEV > case to statically allocate for NR_CPUS since __decrypted has a big > chunk of memory that would be ununsed anyways[1]. And since the second > array is

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 08:54:52AM -0700, Sean Christopherson wrote: > My thought was that we could simply define a second array for the SEV > case to statically allocate for NR_CPUS since __decrypted has a big > chunk of memory that would be ununsed anyways[1]. And since the second > array is

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 08:50 AM, Sean Christopherson wrote: ... So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my book. Why can't you simply free everything in .data..decrypted on !SVE guests? That would prevent adding __decrypted to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
On 09/06/2018 08:50 AM, Sean Christopherson wrote: ... So are we going to be defining a decrypted section for every piece of machinery now? That's a bit too much in my book. Why can't you simply free everything in .data..decrypted on !SVE guests? That would prevent adding __decrypted to

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 05:19:38PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 07:56:40AM -0700, Sean Christopherson wrote: > > Wouldn't that result in @hv_clock_boot being incorrectly freed in the > > !SEV case? > > Ok, maybe I'm missing something but why do we need 4K per CPU? Why

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 05:19:38PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 07:56:40AM -0700, Sean Christopherson wrote: > > Wouldn't that result in @hv_clock_boot being incorrectly freed in the > > !SEV case? > > Ok, maybe I'm missing something but why do we need 4K per CPU? Why

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 07:56:40AM -0700, Sean Christopherson wrote: > Wouldn't that result in @hv_clock_boot being incorrectly freed in the > !SEV case? Ok, maybe I'm missing something but why do we need 4K per CPU? Why can't we map all those pages which contain the clock variable, decrypted in

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 07:56:40AM -0700, Sean Christopherson wrote: > Wouldn't that result in @hv_clock_boot being incorrectly freed in the > !SEV case? Ok, maybe I'm missing something but why do we need 4K per CPU? Why can't we map all those pages which contain the clock variable, decrypted in

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 04:43:42PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > > That would prevent adding __decrypted to existing declarations, e.g. > > hv_clock_boot, which would be ugly in its own right. A more generic > > solution

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 04:43:42PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > > That would prevent adding __decrypted to existing declarations, e.g. > > hv_clock_boot, which would be ugly in its own right. A more generic > > solution

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 07:18:25AM -0700, Sean Christopherson wrote: > Oh, and we'd need to make sure __decrypted_exclusive is freed when > !CONFIG_AMD_MEM_ENCRYPT, and preferably !sev_active() since the big > array is used only if SEV is active. This patch unconditionally > defines hv_clock_dec

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 07:18:25AM -0700, Sean Christopherson wrote: > Oh, and we'd need to make sure __decrypted_exclusive is freed when > !CONFIG_AMD_MEM_ENCRYPT, and preferably !sev_active() since the big > array is used only if SEV is active. This patch unconditionally > defines hv_clock_dec

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > That would prevent adding __decrypted to existing declarations, e.g. > hv_clock_boot, which would be ugly in its own right. A more generic > solution would be to add something like __decrypted_exclusive to mark I still don't

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > That would prevent adding __decrypted to existing declarations, e.g. > hv_clock_boot, which would be ugly in its own right. A more generic > solution would be to add something like __decrypted_exclusive to mark I still don't

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > On Thu, Sep 06, 2018 at 02:24:23PM +0200, Borislav Petkov wrote: > > On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > > > Currently, the per-cpu pvclock data is allocated dynamically when > > > cpu >

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 06:50:41AM -0700, Sean Christopherson wrote: > On Thu, Sep 06, 2018 at 02:24:23PM +0200, Borislav Petkov wrote: > > On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > > > Currently, the per-cpu pvclock data is allocated dynamically when > > > cpu >

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > Currently, the per-cpu pvclock data is allocated dynamically when > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > shared between the guest and the hypervisor hence it must be mapped as > unencrypted (ie. C=0)

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > Currently, the per-cpu pvclock data is allocated dynamically when > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > shared between the guest and the hypervisor hence it must be mapped as > unencrypted (ie. C=0)

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 02:24:23PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > > Currently, the per-cpu pvclock data is allocated dynamically when > > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > > shared between the

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Sean Christopherson
On Thu, Sep 06, 2018 at 02:24:23PM +0200, Borislav Petkov wrote: > On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > > Currently, the per-cpu pvclock data is allocated dynamically when > > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > > shared between the

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > Currently, the per-cpu pvclock data is allocated dynamically when > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > shared between the guest and the hypervisor hence it must be mapped as > unencrypted (ie. C=0)

Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Borislav Petkov
On Thu, Sep 06, 2018 at 06:43:02AM -0500, Brijesh Singh wrote: > Currently, the per-cpu pvclock data is allocated dynamically when > cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is > shared between the guest and the hypervisor hence it must be mapped as > unencrypted (ie. C=0)

[PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
Currently, the per-cpu pvclock data is allocated dynamically when cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is shared between the guest and the hypervisor hence it must be mapped as unencrypted (ie. C=0) when SEV is active. When SEV is active, we will be wasting fairly

[PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

2018-09-06 Thread Brijesh Singh
Currently, the per-cpu pvclock data is allocated dynamically when cpu > HVC_BOOT_ARRAY_SIZE. The physical address of this variable is shared between the guest and the hypervisor hence it must be mapped as unencrypted (ie. C=0) when SEV is active. When SEV is active, we will be wasting fairly