Implement ASID emulation. This allows the l1 guest to run the l2 guest using hw ASID. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13/04/2011 09:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> > Implement ASID emulation. > This allows the l1 guest to run the l2 guest using hw ASID. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>This patch will be complex enough without mixing in cleanups as well (e.g., converting sized type decls to all use [u]intN_t style). Split the cleanups from the new mechanism. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Implement ASID emulation. This allows the l1 guest to run the l2 guest using hw ASID. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13/04/2011 11:37, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> > Implement ASID emulation. > This allows the l1 guest to run the l2 guest using hw ASID. > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>First, how much of a win is this compared with what we do currently? Second, the two-asids-per-vcpu allocation scheme in hvm_asid_handle_vmenter() looks broken. I mean, consider this comment: /* When asid generation changed last time when we were * were going to run l1 guest then next_asid <= nv->nv_n2asid. */ I don''t see how you can assert this to be true. Arbitrary generations can have passed, and next_asid incremented to an arbitrary value, since the last time you allocated nv_n2asid. I wouldn''t bother fixing #2 unless there''s a convincing answer for #1. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Rebased due to change in previous patch. On 04/13/11 12:37, Christoph Egger wrote:> > Implement ASID emulation. > This allows the l1 guest to run the l2 guest using hw ASID. > > Signed-off-by: Christoph Egger<Christoph.Egger@amd.com> >-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/13/11 15:27, Keir Fraser wrote:> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> >> Implement ASID emulation. >> This allows the l1 guest to run the l2 guest using hw ASID. >> >> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com> > > First, how much of a win is this compared with what we do currently?We talk about a win of about 1000 cycles per VMRUN and another 1000 cycles per VMEXIT emulation. That''s a speedup of about 10% for each VMRUN and about 20% for each VMEXIT emulation.> Second, the two-asids-per-vcpu allocation scheme in > hvm_asid_handle_vmenter() looks broken. I mean, consider this comment: > /* When asid generation changed last time when we were > * were going to run l1 guest then next_asid<= nv->nv_n2asid. */Oh, the 2nd ''were'' should be removed.> I don''t see how you can assert this to be true. Arbitrary generations can > have passed, and next_asid incremented to an arbitrary value, since the last > time you allocated nv_n2asid.There are different cases to handle: 1. nestedhvm is disabled. In this case, ''run_n2guest'' is always false then the function should have the old behaviour across multiple VMRUNs. 2. nestedhvm is enabled and we are going to run l1 guest We run the l2 guest in the last call. The asid generation may have changed by then. In this case the current nv_n1asid number is stale and the value of data->next_asid is <= of nv->nv_n1asid. The the value of nv->nv_n1asid is valid data->next_asid is larger than nv->nv_n1asid. In this case just reuse the same hw ASID that has been used from the last VMEXIT emulation. 3. nestedhvm is enabled and we are going to run l1 guest again The same hw ASID should be reused unless the generation changed because the nestedp2m got flushed or the vcpu moved to a different physical cpu, for example. But the hw ASID number may never match the hw ASID used to run the l2 guest. 4. nestedhvm is enabled and we are going to run l2 guest We run the l1 guest in the last call. The asid generation may have changed by then. In this case the current nv_n2asid number is stale and the value of data->next_asid is <= of nv->nv_n2asid. The the value of nv->nv_n2asid is valid if l1 guest doesn''t change the virtual asid (= asid number in the virtual vmcb) and data->next_asid is larger than nv->nv_n2asid. In this case just reuse the same hw ASID that has been used from the last VMRUN emulation. 5. nestedhvm is enabled and we are going to run l2 guest again The same hw ASID should be reused unless the generation changed because the nestedp2m got flushed or the vcpu moved to a different physical cpu, for example. But the hw ASID number may never match the hw ASID used to run the l1 guest. In all cases we have to verify that the> I wouldn''t bother fixing #2 unless there''s a convincing answer for #1. > > -- Keir-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/13/11 15:51, Christoph Egger wrote:> Rebased due to change in previous patch.Fixed build fallout from rebasing.> On 04/13/11 12:37, Christoph Egger wrote: >> >> Implement ASID emulation. >> This allows the l1 guest to run the l2 guest using hw ASID. >> >> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13/04/2011 15:26, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> On 04/13/11 15:27, Keir Fraser wrote: >> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >> > We talk about a win of about 1000 cycles per VMRUN and another 1000 > cycles per VMEXIT emulation. > > That''s a speedup of about 10% for each VMRUN and about 20% for each > VMEXIT emulation.Is this measurable on a macro benchmark? I mean this looks like a micro-optimisation on a feature that noone is going to use for serious performance work anyway.> 4. nestedhvm is enabled and we are going to run l2 guest > > We run the l1 guest in the last call. The asid generation may have > changed by then. In this case the current nv_n2asid number is stale > and the value of data->next_asid is <= of nv->nv_n2asid.How do you know for sure that next_asid will be <= nv_n2asid in this case? What if other VCPUs have run meanwhile, and next_asid has been incremented multiple times until it is greather than nv_n2asid? -- Keir> The the value of nv->nv_n2asid is valid if l1 guest doesn''t change > the virtual asid (= asid number in the virtual vmcb) and > data->next_asid is larger than nv->nv_n2asid. In this case > just reuse the same hw ASID that has been used from the last > VMRUN emulation. > > 5. nestedhvm is enabled and we are going to run l2 guest again > > The same hw ASID should be reused unless the generation changed because > the nestedp2m got flushed or the vcpu moved to a different physical cpu, > for example. > But the hw ASID number may never match the hw ASID used to run the l1 guest. > > > In all cases we have to verify that the > >> I wouldn''t bother fixing #2 unless there''s a convincing answer for #1. >> >> -- Keir >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/13/11 17:05, Keir Fraser wrote:> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> On 04/13/11 15:27, Keir Fraser wrote: >>> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >>> >> We talk about a win of about 1000 cycles per VMRUN and another 1000 >> cycles per VMEXIT emulation. >> >> That''s a speedup of about 10% for each VMRUN and about 20% for each >> VMEXIT emulation. > > Is this measurable on a macro benchmark?I measured this with xentrace while l2 guest is booting. The speedup is noticable on the end-user side just by the feeling on how fast the l2 guest reacts on user input.> I mean this looks like a micro-optimisation on a feature that noone is going > to use for serious performance work anyway. > >> 4. nestedhvm is enabled and we are going to run l2 guest >> >> We run the l1 guest in the last call. The asid generation may have >> changed by then. In this case the current nv_n2asid number is stale >> and the value of data->next_asid is<= of nv->nv_n2asid. > > How do you know for sure that next_asid will be<= nv_n2asid in this case? > What if other VCPUs have run meanwhile, and next_asid has been incremented > multiple times until it is greather than nv_n2asid?In that case curr->arch.hvm_vcpu.asid_generation is not equal to data->core_asid_generation and a new hw ASID number is assigned regardless of the values of data->next_asid and nv_n2asid. Christoph> > -- Keir > >> The the value of nv->nv_n2asid is valid if l1 guest doesn''t change >> the virtual asid (= asid number in the virtual vmcb) and >> data->next_asid is larger than nv->nv_n2asid. In this case >> just reuse the same hw ASID that has been used from the last >> VMRUN emulation. >> >> 5. nestedhvm is enabled and we are going to run l2 guest again >> >> The same hw ASID should be reused unless the generation changed because >> the nestedp2m got flushed or the vcpu moved to a different physical cpu, >> for example. >> But the hw ASID number may never match the hw ASID used to run the l1 guest. >> >> >> In all cases we have to verify that the >> >>> I wouldn''t bother fixing #2 unless there''s a convincing answer for #1. >>> >>> -- Keir >> > > >-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13/04/2011 16:19, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> On 04/13/11 17:05, Keir Fraser wrote: >> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >> >> Is this measurable on a macro benchmark? > > I measured this with xentrace while l2 guest is booting. > > The speedup is noticable on the end-user side just by the feeling on how > fast the l2 guest reacts on user input.Yikes. Does nestedhvm suck really badly then? I can''t believe this patch alone gets you from sucky to good performance. Is it an improvement from sucky to not-quite-as-sucky?>> I mean this looks like a micro-optimisation on a feature that noone is going >> to use for serious performance work anyway. >> >>> 4. nestedhvm is enabled and we are going to run l2 guest >>> >>> We run the l1 guest in the last call. The asid generation may have >>> changed by then. In this case the current nv_n2asid number is stale >>> and the value of data->next_asid is<= of nv->nv_n2asid. >> >> How do you know for sure that next_asid will be<= nv_n2asid in this case? >> What if other VCPUs have run meanwhile, and next_asid has been incremented >> multiple times until it is greather than nv_n2asid? > > In that case curr->arch.hvm_vcpu.asid_generation is not equal to > data->core_asid_generation and a new hw ASID number is assigned > regardless of the values of data->next_asid and nv_n2asid.What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid was assigned from the current generation? Then you''d have next_asid > nv_n2asid, but nv_n2asid is stale. -- Keir> Christoph > >> >> -- Keir >> >>> The the value of nv->nv_n2asid is valid if l1 guest doesn''t change >>> the virtual asid (= asid number in the virtual vmcb) and >>> data->next_asid is larger than nv->nv_n2asid. In this case >>> just reuse the same hw ASID that has been used from the last >>> VMRUN emulation. >>> >>> 5. nestedhvm is enabled and we are going to run l2 guest again >>> >>> The same hw ASID should be reused unless the generation changed because >>> the nestedp2m got flushed or the vcpu moved to a different physical cpu, >>> for example. >>> But the hw ASID number may never match the hw ASID used to run the l1 guest. >>> >>> >>> In all cases we have to verify that the >>> >>>> I wouldn''t bother fixing #2 unless there''s a convincing answer for #1. >>>> >>>> -- Keir >>> >> >> >> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/13/11 18:22, Keir Fraser wrote:> On 13/04/2011 16:19, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> On 04/13/11 17:05, Keir Fraser wrote: >>> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >>> >>> Is this measurable on a macro benchmark? >> >> I measured this with xentrace while l2 guest is booting. >> >> The speedup is noticable on the end-user side just by the feeling on how >> fast the l2 guest reacts on user input. > > Yikes. Does nestedhvm suck really badly then? I can''t believe this patch > alone gets you from sucky to good performance. Is it an improvement from > sucky to not-quite-as-sucky?The very first patch series I sent to xen-devel, the cost of a VMRUN emulation was about 25000 cycles with Xen-on-Xen while hvmloader was coming up as l2 guest. With every new patch series there were also performance optimizations done. Now the cost of a VMRUN emulation is about 11000 cycles and with this ASID emulation patch the cost of a VMRUN emulation goes down to about 10000 cycles. There is still room for more performance, I am working on. A cost of 15000 cycles for VMRUN emulation is fast enough that Solitair in XP mode of Windows 7 guest recognizes a double click ;-) Solitair in XP mode runs faster than Solitair in Windows 7 ;-)>>> I mean this looks like a micro-optimisation on a feature that noone is going >>> to use for serious performance work anyway. >>> >>>> 4. nestedhvm is enabled and we are going to run l2 guest >>>> >>>> We run the l1 guest in the last call. The asid generation may have >>>> changed by then. In this case the current nv_n2asid number is stale >>>> and the value of data->next_asid is<= of nv->nv_n2asid. >>> >>> How do you know for sure that next_asid will be<= nv_n2asid in this case? >>> What if other VCPUs have run meanwhile, and next_asid has been incremented >>> multiple times until it is greather than nv_n2asid? >> >> In that case curr->arch.hvm_vcpu.asid_generation is not equal to >> data->core_asid_generation and a new hw ASID number is assigned >> regardless of the values of data->next_asid and nv_n2asid. > > What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid > was assigned from the current generation? Then you''d have next_asid> > nv_n2asid, but nv_n2asid is stale.When the generation changes, next_asid is set back to 1 and the hw TLB flushed. If next_asid is larger than nv_n2asid then it is reused and this is ok since the hw TLB got flushed. It is just important that nv_n1asid never gets the same hw ASID assigned and that is what the do { } while loop ensures. The hardware doesn''t enforce an incremental use of the hw ASID numbers. The point of ASID emulation is to reduce the number of TLB invalidations by switching forth and back between two hw ASID numbers when switching between l1 and l2 guest. This reduces pagetable walks of the hw MMU so much that the performance win is noticable by the end-user. Without this patch the ASID is invalidated on every VMRUN and VMEXIT emulation. The Flush-by-ASID feature even let me reuse the same hw ASID number when the l2 guest wants to flush its TLB. Christoph> -- Keir > >> Christoph >> >>> >>> -- Keir >>> >>>> The the value of nv->nv_n2asid is valid if l1 guest doesn''t change >>>> the virtual asid (= asid number in the virtual vmcb) and >>>> data->next_asid is larger than nv->nv_n2asid. In this case >>>> just reuse the same hw ASID that has been used from the last >>>> VMRUN emulation. >>>> >>>> 5. nestedhvm is enabled and we are going to run l2 guest again >>>> >>>> The same hw ASID should be reused unless the generation changed because >>>> the nestedp2m got flushed or the vcpu moved to a different physical cpu, >>>> for example. >>>> But the hw ASID number may never match the hw ASID used to run the l1 guest. >>>> >>>> >>>> In all cases we have to verify that the >>>> >>>>> I wouldn''t bother fixing #2 unless there''s a convincing answer for #1.-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 14/04/2011 10:26, "Christoph Egger" <Christoph.Egger@amd.com> wrote:>> What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid >> was assigned from the current generation? Then you''d have next_asid> >> nv_n2asid, but nv_n2asid is stale. > > When the generation changes, next_asid is set back to 1 and the hw TLB > flushed. > > If next_asid is larger than nv_n2asid then it is reused and this is ok > since the hw TLB got flushed. It is just important that nv_n1asid never > gets the same hw ASID assigned and that is what the do { } while loop > ensures.What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW asid in this generation as this vcpu''s (now stale, as it''s from a previous generation''s) nv_n2asid? This PCPU can be interleaving execution of other HVM VCPUs after all. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/14/11 12:28, Keir Fraser wrote:> On 14/04/2011 10:26, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >>> What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid >>> was assigned from the current generation? Then you''d have next_asid> >>> nv_n2asid, but nv_n2asid is stale. >> >> When the generation changes, next_asid is set back to 1 and the hw TLB >> flushed. >> >> If next_asid is larger than nv_n2asid then it is reused and this is ok >> since the hw TLB got flushed. It is just important that nv_n1asid never >> gets the same hw ASID assigned and that is what the do { } while loop >> ensures. > > What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW > asid in this generation as this vcpu''s (now stale, as it''s from a previous > generation''s) nv_n2asid? This PCPU can be interleaving execution of other > HVM VCPUs after all.I am not sure if I got you right. You mean what if two vcpus run on one physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() before so that asid_generation and core_asid_generation do not match and a new asid is always assigned. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 14/04/2011 15:01, "Christoph Egger" <Christoph.Egger@amd.com> wrote:>> What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW >> asid in this generation as this vcpu''s (now stale, as it''s from a previous >> generation''s) nv_n2asid? This PCPU can be interleaving execution of other >> HVM VCPUs after all. > > I am not sure if I got you right. You mean what if two vcpus run on one > physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() > before so that asid_generation and core_asid_generation do not match and > a new asid is always assigned.No, it only does that if a given VCPU gets scheduled onto a *different* PCPU than last time it ran. I''ve attached a mostly rewritten version of your patch that is about half the size and I believe has a fighting chance of being correct (however it is only build tested). Give it a look and a spin. -- Keir> Christoph_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/14/11 16:43, Keir Fraser wrote:> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >>> What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW >>> asid in this generation as this vcpu''s (now stale, as it''s from a previous >>> generation''s) nv_n2asid? This PCPU can be interleaving execution of other >>> HVM VCPUs after all. >> >> I am not sure if I got you right. You mean what if two vcpus run on one >> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() >> before so that asid_generation and core_asid_generation do not match and >> a new asid is always assigned. > > No, it only does that if a given VCPU gets scheduled onto a *different* PCPU > than last time it ran. > > I''ve attached a mostly rewritten version of your patch that is about half > the size and I believe has a fighting chance of being correct (however it is > only build tested). Give it a look and a spin.Yes, it is correct. I like the idea to maintain a generation per asid. Please apply it. Thanks for this work. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 15/04/2011 09:20, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> On 04/14/11 16:43, Keir Fraser wrote: >> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >> >>>> What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW >>>> asid in this generation as this vcpu''s (now stale, as it''s from a previous >>>> generation''s) nv_n2asid? This PCPU can be interleaving execution of other >>>> HVM VCPUs after all. >>> >>> I am not sure if I got you right. You mean what if two vcpus run on one >>> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() >>> before so that asid_generation and core_asid_generation do not match and >>> a new asid is always assigned. >> >> No, it only does that if a given VCPU gets scheduled onto a *different* PCPU >> than last time it ran. >> >> I''ve attached a mostly rewritten version of your patch that is about half >> the size and I believe has a fighting chance of being correct (however it is >> only build tested). Give it a look and a spin. > > Yes, it is correct. I like the idea to maintain a generation per asid. > Please apply it. Thanks for this work.You didn''t notice my subtle error in switching n1 and n2 asid selection in svm_asid_handle_vmrun()? ;-) Actually I bet it would take a lot of testing to pick up on that error, since the transposition doesn''t much matter except that we are then switched round relative to which ASID gets flushed on guest-initiated INVLPGA, and also we''d flush the ''wrong'' ASID when guest requests a new L2 guest ASID. Anyway, glad I spotted it before I checked the patch in! Stale TLB bugs are no fun. -- Keir> Christoph >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/15/11 11:05, Keir Fraser wrote:> > > > On 15/04/2011 09:20, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> On 04/14/11 16:43, Keir Fraser wrote: >>> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >>> >>>>> What if some other vcpu''s nv_n1asid or nv_n2asid got assigned the same HW >>>>> asid in this generation as this vcpu''s (now stale, as it''s from a previous >>>>> generation''s) nv_n2asid? This PCPU can be interleaving execution of other >>>>> HVM VCPUs after all. >>>> >>>> I am not sure if I got you right. You mean what if two vcpus run on one >>>> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() >>>> before so that asid_generation and core_asid_generation do not match and >>>> a new asid is always assigned. >>> >>> No, it only does that if a given VCPU gets scheduled onto a *different* PCPU >>> than last time it ran. >>> >>> I''ve attached a mostly rewritten version of your patch that is about half >>> the size and I believe has a fighting chance of being correct (however it is >>> only build tested). Give it a look and a spin. >> >> Yes, it is correct. I like the idea to maintain a generation per asid. >> Please apply it. Thanks for this work. > > You didn''t notice my subtle error in switching n1 and n2 asid selection in > svm_asid_handle_vmrun()? ;-)Oh, now that you mention it... A l2 smp guest actually boots up. Maybe I need to use more vcpus than I have physical cpus to hit that bug. I think, this snippet should be like this: struct vcpu *curr = current; struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb; struct hvm_vcpu_asid *curr_asid; bool_t need_flush; bool_t vcpu_guestmode = 0; if (nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr)) vcpu_guestmode = 1; curr_asid = vcpu_guestmode ? &curr->arch.hvm_vcpu.n2_asid : &curr->arch.hvm_vcpu.n1_asid; need_flush = hvm_asid_handle_vmenter(curr_asid); Christoph> Actually I bet it would take a lot of testing to pick up on that error,> since the transposition doesn''t much matter except> that we are then switched round relative to which ASID gets flushed on > guest-initiated INVLPGA, and also we''d flush the ''wrong'' ASID when guest > requests a new L2 guest ASID. Anyway, glad I spotted it before I checked the > patch in! Stale TLB bugs are no fun. > > -- Keir > >> Christoph >> > > >-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 15/04/2011 10:08, "Christoph Egger" <Christoph.Egger@amd.com> wrote:>> You didn''t notice my subtle error in switching n1 and n2 asid selection in >> svm_asid_handle_vmrun()? ;-) > > Oh, now that you mention it...Yeah, I fixed it before I checked it in (xen-unstable:23229).> A l2 smp guest actually boots up. Maybe I need to use more vcpus than I > have physical cpus to hit that bug.I think it would have been a source of very subtle quite hard-to-repro bugs. Not something you''d pick up in a simple does-it-boot smoke test. So a pretty nasty bug all round! -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/15/11 11:24, Keir Fraser wrote:> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >>> You didn''t notice my subtle error in switching n1 and n2 asid selection in >>> svm_asid_handle_vmrun()? ;-) >> >> Oh, now that you mention it... > > Yeah, I fixed it before I checked it in (xen-unstable:23229). > >> A l2 smp guest actually boots up. Maybe I need to use more vcpus than I >> have physical cpus to hit that bug. > > I think it would have been a source of very subtle quite hard-to-repro bugs. > Not something you''d pick up in a simple does-it-boot smoke test. So a pretty > nasty bug all round!Just found another fallout that got lost from my original patch. After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER after about 30 seconds. When starting XP mode again, Win 7 turns SVM on again. Then nv_n2asid can be stale. Attached patch fixes this. Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/15/11 14:53, Keir Fraser wrote:> On 15/04/2011 10:57, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> On 04/15/11 11:24, Keir Fraser wrote: >>> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >> >> Just found another fallout that got lost from my original patch. >> >> After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER >> after about 30 seconds. When starting XP mode again, Win 7 turns SVM on >> again. >> >> Then nv_n2asid can be stale. Attached patch fixes this. > > Should be using hvm_asid_flush_vcpu_asid(), so I''ll fix the patch to use > that and then check it in.Yes, right. Thank you. Christoph> Thanks, > Keir > >> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com> >> > > >-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 15/04/2011 10:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:> On 04/15/11 11:24, Keir Fraser wrote: >> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > > Just found another fallout that got lost from my original patch. > > After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER > after about 30 seconds. When starting XP mode again, Win 7 turns SVM on > again. > > Then nv_n2asid can be stale. Attached patch fixes this.Should be using hvm_asid_flush_vcpu_asid(), so I''ll fix the patch to use that and then check it in. Thanks, Keir> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 04/15/11 14:53, Keir Fraser wrote:> On 15/04/2011 10:57, "Christoph Egger"<Christoph.Egger@amd.com> wrote: > >> On 04/15/11 11:24, Keir Fraser wrote: >>> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com> wrote: >> >> Just found another fallout that got lost from my original patch. >> >> After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER >> after about 30 seconds. When starting XP mode again, Win 7 turns SVM on >> again. >> >> Then nv_n2asid can be stale. Attached patch fixes this. > > Should be using hvm_asid_flush_vcpu_asid(), so I''ll fix the patch to use > that and then check it in.There are a lot of regressions now... I assume the non-nested virtualization case got broken. Attached patch covers the non-nested case (and contains the above discussed hvm_asid_flush_vcpu_asid() as it didn''t appear yet upstream). Signed-off-by: Christoph Egger<Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel