| Message ID | 20170214194259.75960-4-thgarnie@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: > The KVM segment_base function is confusing. This patch replaces integers > with appropriate flags, simplify constructs and add comments. It could pay to put this first in the series, but last is probably fine, too. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20170213 > --- > arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 99167f20bc34..edb8326108dd 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) > struct desc_struct *d; > unsigned long table_base; This should go away IMO. It should be struct desc_struct *table; > + table_base = get_current_gdt_rw_vaddr(); Then this can go away, too, and you can stop having the silly get_current_gdt_rw_vaddr() function at all. > + d = (struct desc_struct *)table_base + (selector >> 3); And this cast goes away too.
On Tue, Feb 14, 2017 at 7:57 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: >> The KVM segment_base function is confusing. This patch replaces integers >> with appropriate flags, simplify constructs and add comments. > > It could pay to put this first in the series, but last is probably fine, too. > :) >> >> Signed-off-by: Thomas Garnier <thgarnie@google.com> >> --- >> Based on next-20170213 >> --- >> arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- >> 1 file changed, 18 insertions(+), 8 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 99167f20bc34..edb8326108dd 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) >> struct desc_struct *d; >> unsigned long table_base; > > This should go away IMO. It should be struct desc_struct *table; > >> + table_base = get_current_gdt_rw_vaddr(); > > Then this can go away, too, and you can stop having the silly > get_current_gdt_rw_vaddr() function at all. > >> + d = (struct desc_struct *)table_base + (selector >> 3); > > And this cast goes away too. No problem.
Can we use the read-only GDT here? When expanding the virtual address for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 && d->type != 0)? On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: > The KVM segment_base function is confusing. This patch replaces integers > with appropriate flags, simplify constructs and add comments. > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > --- > Based on next-20170213 > --- > arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 99167f20bc34..edb8326108dd 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) > struct desc_struct *d; > unsigned long table_base; > unsigned long v; > + u32 high32; > > - if (!(selector & ~3)) > + if (!(selector & ~SEGMENT_RPL_MASK)) > return 0; > > - table_base = get_current_gdt_rw_vaddr(); > - > - if (selector & 4) { /* from ldt */ > + /* LDT selector */ > + if ((selector & SEGMENT_TI_MASK) == SEGMENT_LDT) { > u16 ldt_selector = kvm_read_ldt(); > > - if (!(ldt_selector & ~3)) > + if (!(ldt_selector & ~SEGMENT_RPL_MASK)) > return 0; > > table_base = segment_base(ldt_selector); > + } else { > + table_base = get_current_gdt_rw_vaddr(); > } > - d = (struct desc_struct *)(table_base + (selector & ~7)); > + > + d = (struct desc_struct *)table_base + (selector >> 3); > v = get_desc_base(d); > #ifdef CONFIG_X86_64 > - if (d->s == 0 && (d->type == 2 || d->type == 9 || d->type == 11)) > - v |= ((unsigned long)((struct ldttss_desc64 *)d)->base3) << 32; > + /* > + * Extend the virtual address if we have a system descriptor entry for > + * LDT or TSS (available or busy). > + */ > + if (d->s == 0 && (d->type == DESC_LDT || d->type == DESC_TSS || > + d->type == 11/*Busy TSS */)) { > + high32 = ((struct ldttss_desc64 *)d)->base3; > + v |= (u64)high32 << 32; > + } > #endif > return v; > } > -- > 2.11.0.483.g087da7b7c-goog >
On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: > > Can we use the read-only GDT here? When expanding the virtual address > for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 > && d->type != 0)? We can use the readonly GDT but I think doesn't matter one or the other here. We have to check specific types for LDT or TSS, other values describe other entries (cf Intel volume 3, 3.5) (for example 14 & 15 on 64-bits are for trap & interrupt gates). > > > On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: > > The KVM segment_base function is confusing. This patch replaces integers > > with appropriate flags, simplify constructs and add comments. > > > > Signed-off-by: Thomas Garnier <thgarnie@google.com> > > --- > > Based on next-20170213 > > --- > > arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- > > 1 file changed, 18 insertions(+), 8 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 99167f20bc34..edb8326108dd 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) > > struct desc_struct *d; > > unsigned long table_base; > > unsigned long v; > > + u32 high32; > > > > - if (!(selector & ~3)) > > + if (!(selector & ~SEGMENT_RPL_MASK)) > > return 0; > > > > - table_base = get_current_gdt_rw_vaddr(); > > - > > - if (selector & 4) { /* from ldt */ > > + /* LDT selector */ > > + if ((selector & SEGMENT_TI_MASK) == SEGMENT_LDT) { > > u16 ldt_selector = kvm_read_ldt(); > > > > - if (!(ldt_selector & ~3)) > > + if (!(ldt_selector & ~SEGMENT_RPL_MASK)) > > return 0; > > > > table_base = segment_base(ldt_selector); > > + } else { > > + table_base = get_current_gdt_rw_vaddr(); > > } > > - d = (struct desc_struct *)(table_base + (selector & ~7)); > > + > > + d = (struct desc_struct *)table_base + (selector >> 3); > > v = get_desc_base(d); > > #ifdef CONFIG_X86_64 > > - if (d->s == 0 && (d->type == 2 || d->type == 9 || d->type == 11)) > > - v |= ((unsigned long)((struct ldttss_desc64 *)d)->base3) << 32; > > + /* > > + * Extend the virtual address if we have a system descriptor entry for > > + * LDT or TSS (available or busy). > > + */ > > + if (d->s == 0 && (d->type == DESC_LDT || d->type == DESC_TSS || > > + d->type == 11/*Busy TSS */)) { > > + high32 = ((struct ldttss_desc64 *)d)->base3; > > + v |= (u64)high32 << 32; > > + } > > #endif > > return v; > > } > > -- > > 2.11.0.483.g087da7b7c-goog > >
On Fri, Feb 17, 2017 at 12:11 PM, Thomas Garnier <thgarnie@google.com> wrote: > On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: >> >> Can we use the read-only GDT here? When expanding the virtual address >> for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 >> && d->type != 0)? > > We can use the readonly GDT but I think doesn't matter one or the > other here. We have to check specific types for LDT or TSS, other > values describe other entries (cf Intel volume 3, 3.5) (for example 14 > & 15 on 64-bits are for trap & interrupt gates). According to volume 3 of the SDM, section 3.5.2: The following system descriptors expand to 16 bytes: — Call gate descriptors (see Section 5.8.3.1, “IA-32e Mode Call Gates”) — IDT gate descriptors (see Section 6.14.1, “64-Bit Mode IDT”) — LDT and TSS descriptors (see Section 7.2.3, “TSS Descriptor in 64-bit mode”). All legal system descriptor types (except for 0: Upper 8 bytes of an 16-byte descriptor) should get the high 32 bits of the base address from the next 8-byte descriptor. > >> >> >> On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: >> > The KVM segment_base function is confusing. This patch replaces integers >> > with appropriate flags, simplify constructs and add comments. >> > >> > Signed-off-by: Thomas Garnier <thgarnie@google.com> >> > --- >> > Based on next-20170213 >> > --- >> > arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- >> > 1 file changed, 18 insertions(+), 8 deletions(-) >> > >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> > index 99167f20bc34..edb8326108dd 100644 >> > --- a/arch/x86/kvm/vmx.c >> > +++ b/arch/x86/kvm/vmx.c >> > @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) >> > struct desc_struct *d; >> > unsigned long table_base; >> > unsigned long v; >> > + u32 high32; >> > >> > - if (!(selector & ~3)) >> > + if (!(selector & ~SEGMENT_RPL_MASK)) >> > return 0; >> > >> > - table_base = get_current_gdt_rw_vaddr(); >> > - >> > - if (selector & 4) { /* from ldt */ >> > + /* LDT selector */ >> > + if ((selector & SEGMENT_TI_MASK) == SEGMENT_LDT) { >> > u16 ldt_selector = kvm_read_ldt(); >> > >> > - if (!(ldt_selector & ~3)) >> > + if (!(ldt_selector & ~SEGMENT_RPL_MASK)) >> > return 0; >> > >> > table_base = segment_base(ldt_selector); >> > + } else { >> > + table_base = get_current_gdt_rw_vaddr(); >> > } >> > - d = (struct desc_struct *)(table_base + (selector & ~7)); >> > + >> > + d = (struct desc_struct *)table_base + (selector >> 3); >> > v = get_desc_base(d); >> > #ifdef CONFIG_X86_64 >> > - if (d->s == 0 && (d->type == 2 || d->type == 9 || d->type == 11)) >> > - v |= ((unsigned long)((struct ldttss_desc64 *)d)->base3) << 32; >> > + /* >> > + * Extend the virtual address if we have a system descriptor entry for >> > + * LDT or TSS (available or busy). >> > + */ >> > + if (d->s == 0 && (d->type == DESC_LDT || d->type == DESC_TSS || >> > + d->type == 11/*Busy TSS */)) { >> > + high32 = ((struct ldttss_desc64 *)d)->base3; >> > + v |= (u64)high32 << 32; >> > + } >> > #endif >> > return v; >> > } >> > -- >> > 2.11.0.483.g087da7b7c-goog >> > > > > > > -- > Thomas
On Fri, Feb 17, 2017 at 1:00 PM, Jim Mattson <jmattson@google.com> wrote: > On Fri, Feb 17, 2017 at 12:11 PM, Thomas Garnier <thgarnie@google.com> wrote: >> On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: >>> >>> Can we use the read-only GDT here? When expanding the virtual address >>> for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 >>> && d->type != 0)? >> >> We can use the readonly GDT but I think doesn't matter one or the >> other here. We have to check specific types for LDT or TSS, other >> values describe other entries (cf Intel volume 3, 3.5) (for example 14 >> & 15 on 64-bits are for trap & interrupt gates). > > According to volume 3 of the SDM, section 3.5.2: > > The following system descriptors expand to 16 bytes: > — Call gate descriptors (see Section 5.8.3.1, “IA-32e Mode Call Gates”) > — IDT gate descriptors (see Section 6.14.1, “64-Bit Mode IDT”) > — LDT and TSS descriptors (see Section 7.2.3, “TSS Descriptor in 64-bit mode”). > > All legal system descriptor types (except for 0: Upper 8 bytes of an 16-byte > descriptor) should get the high 32 bits of the base address from the next 8-byte > descriptor. > Ok, then I will test an updated version next week. >> >>> >>> >>> On Tue, Feb 14, 2017 at 11:42 AM, Thomas Garnier <thgarnie@google.com> wrote: >>> > The KVM segment_base function is confusing. This patch replaces integers >>> > with appropriate flags, simplify constructs and add comments. >>> > >>> > Signed-off-by: Thomas Garnier <thgarnie@google.com> >>> > --- >>> > Based on next-20170213 >>> > --- >>> > arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- >>> > 1 file changed, 18 insertions(+), 8 deletions(-) >>> > >>> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> > index 99167f20bc34..edb8326108dd 100644 >>> > --- a/arch/x86/kvm/vmx.c >>> > +++ b/arch/x86/kvm/vmx.c >>> > @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) >>> > struct desc_struct *d; >>> > unsigned long table_base; >>> > unsigned long v; >>> > + u32 high32; >>> > >>> > - if (!(selector & ~3)) >>> > + if (!(selector & ~SEGMENT_RPL_MASK)) >>> > return 0; >>> > >>> > - table_base = get_current_gdt_rw_vaddr(); >>> > - >>> > - if (selector & 4) { /* from ldt */ >>> > + /* LDT selector */ >>> > + if ((selector & SEGMENT_TI_MASK) == SEGMENT_LDT) { >>> > u16 ldt_selector = kvm_read_ldt(); >>> > >>> > - if (!(ldt_selector & ~3)) >>> > + if (!(ldt_selector & ~SEGMENT_RPL_MASK)) >>> > return 0; >>> > >>> > table_base = segment_base(ldt_selector); >>> > + } else { >>> > + table_base = get_current_gdt_rw_vaddr(); >>> > } >>> > - d = (struct desc_struct *)(table_base + (selector & ~7)); >>> > + >>> > + d = (struct desc_struct *)table_base + (selector >> 3); >>> > v = get_desc_base(d); >>> > #ifdef CONFIG_X86_64 >>> > - if (d->s == 0 && (d->type == 2 || d->type == 9 || d->type == 11)) >>> > - v |= ((unsigned long)((struct ldttss_desc64 *)d)->base3) << 32; >>> > + /* >>> > + * Extend the virtual address if we have a system descriptor entry for >>> > + * LDT or TSS (available or busy). >>> > + */ >>> > + if (d->s == 0 && (d->type == DESC_LDT || d->type == DESC_TSS || >>> > + d->type == 11/*Busy TSS */)) { >>> > + high32 = ((struct ldttss_desc64 *)d)->base3; >>> > + v |= (u64)high32 << 32; >>> > + } >>> > #endif >>> > return v; >>> > } >>> > -- >>> > 2.11.0.483.g087da7b7c-goog >>> > >> >> >> >> >> -- >> Thomas
On Fri, Feb 17, 2017 at 2:01 PM, Thomas Garnier <thgarnie@google.com> wrote: > On Fri, Feb 17, 2017 at 1:00 PM, Jim Mattson <jmattson@google.com> wrote: >> On Fri, Feb 17, 2017 at 12:11 PM, Thomas Garnier <thgarnie@google.com> wrote: >>> On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: >>>> >>>> Can we use the read-only GDT here? When expanding the virtual address >>>> for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 >>>> && d->type != 0)? >>> >>> We can use the readonly GDT but I think doesn't matter one or the >>> other here. We have to check specific types for LDT or TSS, other >>> values describe other entries (cf Intel volume 3, 3.5) (for example 14 >>> & 15 on 64-bits are for trap & interrupt gates). >> >> According to volume 3 of the SDM, section 3.5.2: >> >> The following system descriptors expand to 16 bytes: >> — Call gate descriptors (see Section 5.8.3.1, “IA-32e Mode Call Gates”) >> — IDT gate descriptors (see Section 6.14.1, “64-Bit Mode IDT”) >> — LDT and TSS descriptors (see Section 7.2.3, “TSS Descriptor in 64-bit mode”). >> >> All legal system descriptor types (except for 0: Upper 8 bytes of an 16-byte >> descriptor) should get the high 32 bits of the base address from the next 8-byte >> descriptor. >> > > Ok, then I will test an updated version next week. > I'm going to send out some preliminary patches that just get rid of this problem entirely.
On Mon, Feb 20, 2017 at 8:56 AM, Andy Lutomirski <luto@amacapital.net> wrote: > On Fri, Feb 17, 2017 at 2:01 PM, Thomas Garnier <thgarnie@google.com> wrote: >> On Fri, Feb 17, 2017 at 1:00 PM, Jim Mattson <jmattson@google.com> wrote: >>> On Fri, Feb 17, 2017 at 12:11 PM, Thomas Garnier <thgarnie@google.com> wrote: >>>> On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: >>>>> >>>>> Can we use the read-only GDT here? When expanding the virtual address >>>>> for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 >>>>> && d->type != 0)? >>>> >>>> We can use the readonly GDT but I think doesn't matter one or the >>>> other here. We have to check specific types for LDT or TSS, other >>>> values describe other entries (cf Intel volume 3, 3.5) (for example 14 >>>> & 15 on 64-bits are for trap & interrupt gates). >>> >>> According to volume 3 of the SDM, section 3.5.2: >>> >>> The following system descriptors expand to 16 bytes: >>> — Call gate descriptors (see Section 5.8.3.1, “IA-32e Mode Call Gates”) >>> — IDT gate descriptors (see Section 6.14.1, “64-Bit Mode IDT”) >>> — LDT and TSS descriptors (see Section 7.2.3, “TSS Descriptor in 64-bit mode”). >>> >>> All legal system descriptor types (except for 0: Upper 8 bytes of an 16-byte >>> descriptor) should get the high 32 bits of the base address from the next 8-byte >>> descriptor. >>> >> >> Ok, then I will test an updated version next week. >> > > I'm going to send out some preliminary patches that just get rid of > this problem entirely. Okay, I guess I will have to wait for it to be integrated to linux-next then. Or would you rather to it after this patch set is added?
On Mon, Feb 20, 2017 at 9:28 AM, Thomas Garnier <thgarnie@google.com> wrote: > On Mon, Feb 20, 2017 at 8:56 AM, Andy Lutomirski <luto@amacapital.net> wrote: >> On Fri, Feb 17, 2017 at 2:01 PM, Thomas Garnier <thgarnie@google.com> wrote: >>> On Fri, Feb 17, 2017 at 1:00 PM, Jim Mattson <jmattson@google.com> wrote: >>>> On Fri, Feb 17, 2017 at 12:11 PM, Thomas Garnier <thgarnie@google.com> wrote: >>>>> On Fri, Feb 17, 2017 at 9:49 AM, Jim Mattson <jmattson@google.com> wrote: >>>>>> >>>>>> Can we use the read-only GDT here? When expanding the virtual address >>>>>> for 64-bit system descriptors, isn't it sufficient to check (d->s == 0 >>>>>> && d->type != 0)? >>>>> >>>>> We can use the readonly GDT but I think doesn't matter one or the >>>>> other here. We have to check specific types for LDT or TSS, other >>>>> values describe other entries (cf Intel volume 3, 3.5) (for example 14 >>>>> & 15 on 64-bits are for trap & interrupt gates). >>>> >>>> According to volume 3 of the SDM, section 3.5.2: >>>> >>>> The following system descriptors expand to 16 bytes: >>>> — Call gate descriptors (see Section 5.8.3.1, “IA-32e Mode Call Gates”) >>>> — IDT gate descriptors (see Section 6.14.1, “64-Bit Mode IDT”) >>>> — LDT and TSS descriptors (see Section 7.2.3, “TSS Descriptor in 64-bit mode”). >>>> >>>> All legal system descriptor types (except for 0: Upper 8 bytes of an 16-byte >>>> descriptor) should get the high 32 bits of the base address from the next 8-byte >>>> descriptor. >>>> >>> >>> Ok, then I will test an updated version next week. >>> >> >> I'm going to send out some preliminary patches that just get rid of >> this problem entirely. > > Okay, I guess I will have to wait for it to be integrated to > linux-next then. Or would you rather to it after this patch set is > added? > Read your summary for the patchset of KVM cleanup, I will wait for it to reach linux-next to rebase and send the new iteration. Thanks for working on the clean-up. > -- > Thomas
* Thomas Garnier <thgarnie@google.com> wrote: > > Okay, I guess I will have to wait for it to be integrated to > > linux-next then. Or would you rather to it after this patch set is > > added? > > Read your summary for the patchset of KVM cleanup, I will wait for it to reach > linux-next to rebase and send the new iteration. Note that to be able to apply your readonly-GDT series to the x86 tree I'll need a stable SHA1 to base it on. Paolo, how stable, non-rebasing are the KVM tree commits? Once Andy's patches hit the KVM tree I could pull from you and order it so that I'd send it to Linus only after you sent your bits upstream. That would save us 3 months of patch propagation latency. Or should we keep Andy's KVM patches together with the GDT patches? Either workflow works for me - it's your call as these are predominantly KVM changes. Thanks, Ingo
> Paolo, how stable, non-rebasing are the KVM tree commits? Whatever ends in linux-next is stable. I have a separate rebasing branch, but it's not part of linux-next by design. > Or should we keep Andy's KVM patches together with the GDT patches? Either > workflow works for me - it's your call as these are predominantly KVM > changes. I'll delay my pull request to Linus a couple days so that I can test Andy's 6 patches. Then you can just base your branch on Linus's tree. Paolo
* Paolo Bonzini <pbonzini@redhat.com> wrote: > > Paolo, how stable, non-rebasing are the KVM tree commits? > > Whatever ends in linux-next is stable. I have a separate rebasing branch, > but it's not part of linux-next by design. Ok, that's nice! > > Or should we keep Andy's KVM patches together with the GDT patches? Either > > workflow works for me - it's your call as these are predominantly KVM changes. > > I'll delay my pull request to Linus a couple days so that I can test Andy's 6 > patches. Then you can just base your branch on Linus's tree. Fantastic, thank you! Ingo
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 99167f20bc34..edb8326108dd 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2062,25 +2062,35 @@ static unsigned long segment_base(u16 selector) struct desc_struct *d; unsigned long table_base; unsigned long v; + u32 high32; - if (!(selector & ~3)) + if (!(selector & ~SEGMENT_RPL_MASK)) return 0; - table_base = get_current_gdt_rw_vaddr(); - - if (selector & 4) { /* from ldt */ + /* LDT selector */ + if ((selector & SEGMENT_TI_MASK) == SEGMENT_LDT) { u16 ldt_selector = kvm_read_ldt(); - if (!(ldt_selector & ~3)) + if (!(ldt_selector & ~SEGMENT_RPL_MASK)) return 0; table_base = segment_base(ldt_selector); + } else { + table_base = get_current_gdt_rw_vaddr(); } - d = (struct desc_struct *)(table_base + (selector & ~7)); + + d = (struct desc_struct *)table_base + (selector >> 3); v = get_desc_base(d); #ifdef CONFIG_X86_64 - if (d->s == 0 && (d->type == 2 || d->type == 9 || d->type == 11)) - v |= ((unsigned long)((struct ldttss_desc64 *)d)->base3) << 32; + /* + * Extend the virtual address if we have a system descriptor entry for + * LDT or TSS (available or busy). + */ + if (d->s == 0 && (d->type == DESC_LDT || d->type == DESC_TSS || + d->type == 11/*Busy TSS */)) { + high32 = ((struct ldttss_desc64 *)d)->base3; + v |= (u64)high32 << 32; + } #endif return v; }
The KVM segment_base function is confusing. This patch replaces integers with appropriate flags, simplify constructs and add comments. Signed-off-by: Thomas Garnier <thgarnie@google.com> --- Based on next-20170213 --- arch/x86/kvm/vmx.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)