Currently, Xen always starts the ASID allocation at 1. But
for SEV technologies the ASID space is divided. This is
because it's a security issue if a guest is started as
ES/SNP and is migrated to SEV-only. So, the types are
tracked explicitly.
Thus, in preparation of SEV support in Xen, add min_asid
to allow supplying the dynamic start ASID during the
allocation process.
Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
xen/arch/x86/hvm/asid.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/hvm/asid.c b/xen/arch/x86/hvm/asid.c
index 8d27b7dba1..e14b64f2c8 100644
--- a/xen/arch/x86/hvm/asid.c
+++ b/xen/arch/x86/hvm/asid.c
@@ -41,6 +41,7 @@ boolean_param("asid", opt_asid_enabled);
/* Per-CPU ASID management. */
struct hvm_asid_data {
uint64_t core_asid_generation;
+ uint32_t min_asid;
uint32_t next_asid;
uint32_t max_asid;
bool disabled;
@@ -53,7 +54,8 @@ void hvm_asid_init(int nasids)
static int8_t g_disabled = -1;
struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
- data->max_asid = nasids - 1;
+ data->min_asid = 1;
+ data->max_asid = nasids - data->min_asid;
data->disabled = !opt_asid_enabled || (nasids <= 1);
if ( g_disabled != data->disabled )
@@ -66,8 +68,8 @@ void hvm_asid_init(int nasids)
/* Zero indicates 'invalid generation', so we start the count at one. */
data->core_asid_generation = 1;
- /* Zero indicates 'ASIDs disabled', so we start the count at one. */
- data->next_asid = 1;
+ /* Zero indicates 'ASIDs disabled', so we start the count at min_asid. */
+ data->next_asid = data->min_asid;
}
void hvm_asid_flush_vcpu_asid(struct hvm_vcpu_asid *asid)
@@ -117,7 +119,7 @@ bool hvm_asid_handle_vmenter(struct hvm_vcpu_asid *asid)
if ( unlikely(data->next_asid > data->max_asid) )
{
hvm_asid_flush_core();
- data->next_asid = 1;
+ data->next_asid = data->min_asid;
if ( data->disabled )
goto disabled;
}
--
2.44.0
On 16/04/2024 9:54 am, Vaishali Thakkar wrote: > Currently, Xen always starts the ASID allocation at 1. But > for SEV technologies the ASID space is divided. This is > because it's a security issue if a guest is started as > ES/SNP and is migrated to SEV-only. So, the types are > tracked explicitly. > > Thus, in preparation of SEV support in Xen, add min_asid > to allow supplying the dynamic start ASID during the > allocation process. > > Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech> Mechanically, this is fine, but is it going to be useful in the long term? For SEV and SEV-ES/SNP, we must run the VM in the single fixed ASID negotiated with the ASP. This is not not optional. For non-encrypted VMs, we are free to choose between using the remaining ASID space as we used to (i.e. run it per-pCPU and tick it over to flush the TLBs), or to run it in a fixed ASID per guest too. The latter lets us use INVLPGB, and would avoid maintaining two different TLB-tagging schemes. I'd say that we absolutely do want INVLPGB support for managing non-encrypted VMs, and we cannot mix both fixed and floating ASIDs at the same time. We should seriously consider whether it's worth maintaining two schemes, or just switching wholesale to a fixed ASID scheme. I don't have a good answer here... If it where anything but TLB flushing, it would be an obvious choice, but TLB handling bugs are some of the nastiest to show up. ~Andrew
On 4/16/24 4:12 PM, Andrew Cooper wrote: > On 16/04/2024 9:54 am, Vaishali Thakkar wrote: >> Currently, Xen always starts the ASID allocation at 1. But >> for SEV technologies the ASID space is divided. This is >> because it's a security issue if a guest is started as >> ES/SNP and is migrated to SEV-only. So, the types are >> tracked explicitly. >> >> Thus, in preparation of SEV support in Xen, add min_asid >> to allow supplying the dynamic start ASID during the >> allocation process. >> >> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech> > > Mechanically, this is fine, but is it going to be useful in the long term? > > For SEV and SEV-ES/SNP, we must run the VM in the single fixed ASID > negotiated with the ASP. This is not not optional. > > For non-encrypted VMs, we are free to choose between using the remaining > ASID space as we used to (i.e. run it per-pCPU and tick it over to flush > the TLBs), or to run it in a fixed ASID per guest too. > > The latter lets us use INVLPGB, and would avoid maintaining two > different TLB-tagging schemes. > > > I'd say that we absolutely do want INVLPGB support for managing > non-encrypted VMs, and we cannot mix both fixed and floating ASIDs at > the same time. I agree that having a both schemes at the time is not practical. And I've some RFC patches in work to propose fixed ASID scheme for all domains (encrypted or not) to start a discussion regarding that. IMO, this patch is still useful because my idea was to then use min_asid as a holder to separate out the non-encrypted, encrypted space and SEV ES/ SNP ASID space. If it's more easier to see the usefulness of this patch along with other asid fixes, I'm fine with putting it in that RFC patchset. But I thought that this change was independent enough to be sent by itself. > We should seriously consider whether it's worth maintaining two schemes, > or just switching wholesale to a fixed ASID scheme. > > I don't have a good answer here... If it where anything but TLB > flushing, it would be an obvious choice, but TLB handling bugs are some > of the nastiest to show up. > > ~Andrew
On 4/16/24 4:25 PM, Vaishali Thakkar wrote: > On 4/16/24 4:12 PM, Andrew Cooper wrote: >> On 16/04/2024 9:54 am, Vaishali Thakkar wrote: >>> Currently, Xen always starts the ASID allocation at 1. But >>> for SEV technologies the ASID space is divided. This is >>> because it's a security issue if a guest is started as >>> ES/SNP and is migrated to SEV-only. So, the types are >>> tracked explicitly. >>> >>> Thus, in preparation of SEV support in Xen, add min_asid >>> to allow supplying the dynamic start ASID during the >>> allocation process. >>> >>> Signed-off-by: Vaishali Thakkar <vaishali.thakkar@vates.tech> >> >> Mechanically, this is fine, but is it going to be useful in the long term? >> >> For SEV and SEV-ES/SNP, we must run the VM in the single fixed ASID >> negotiated with the ASP. This is not not optional. >> >> For non-encrypted VMs, we are free to choose between using the remaining >> ASID space as we used to (i.e. run it per-pCPU and tick it over to flush >> the TLBs), or to run it in a fixed ASID per guest too. >> >> The latter lets us use INVLPGB, and would avoid maintaining two >> different TLB-tagging schemes. >> >> >> I'd say that we absolutely do want INVLPGB support for managing >> non-encrypted VMs, and we cannot mix both fixed and floating ASIDs at >> the same time. > > I agree that having a both schemes at the time is not practical. And I've > some RFC patches in work to propose fixed ASID scheme for all domains > (encrypted or not) to start a discussion regarding that. > > IMO, this patch is still useful because my idea was to then use min_asid > as a holder to separate out the non-encrypted, encrypted space and SEV ES/ > SNP ASID space. If it's more easier to see the usefulness of this patch > along with other asid fixes, I'm fine with putting it in that RFC patchset. > But I thought that this change was independent enough to be sent by > itself. s/encrypted/SEV >> We should seriously consider whether it's worth maintaining two schemes, >> or just switching wholesale to a fixed ASID scheme. >> >> I don't have a good answer here... If it where anything but TLB >> flushing, it would be an obvious choice, but TLB handling bugs are some >> of the nastiest to show up. >> >> ~Andrew > >
© 2016 - 2024 Red Hat, Inc.