| Message ID | 20240819074052.207783-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-9.1] target/i386: Fix tss access size in switch_tss_ra | expand |
On Mon, 19 Aug 2024 at 08:42, Richard Henderson <richard.henderson@linaro.org> wrote: > > The two limit_max variables represent size - 1, just like the > encoding in the GDT, thus the 'old' access was off by one. > Access the minimal size of the new tss: the complete tss contains > the iopb, which may be a larger block than the access api expects, > and irrelevant because the iopb is not accessed during the > switch itself. > > Fixes: 8b131065080a ("target/i386/tcg: use X86Access for TSS access") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2511 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/tcg/seg_helper.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Not an x86 expert but this looks OK based on description and what we were doing before 8b131065080a... Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 8/19/24 00:40, Richard Henderson wrote: > The two limit_max variables represent size - 1, just like the > encoding in the GDT, thus the 'old' access was off by one. > Access the minimal size of the new tss: the complete tss contains > the iopb, which may be a larger block than the access api expects, > and irrelevant because the iopb is not accessed during the > switch itself. > > Fixes: 8b131065080a ("target/i386/tcg: use X86Access for TSS access") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2511 > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/i386/tcg/seg_helper.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c > index bab552cd53..3b8fd827e1 100644 > --- a/target/i386/tcg/seg_helper.c > +++ b/target/i386/tcg/seg_helper.c > @@ -378,7 +378,7 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector, > > /* X86Access avoids memory exceptions during the task switch */ > mmu_index = cpu_mmu_index_kernel(env); > - access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max, > + access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max + 1, > MMU_DATA_STORE, mmu_index, retaddr); > > if (source == SWITCH_TSS_CALL) { > @@ -386,7 +386,8 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector, > probe_access(env, tss_base, 2, MMU_DATA_STORE, > mmu_index, retaddr); > } > - access_prepare_mmu(&new, env, tss_base, tss_limit, > + /* While true tss_limit may be larger, we don't access the iopb here. */ > + access_prepare_mmu(&new, env, tss_base, tss_limit_max + 1, > MMU_DATA_LOAD, mmu_index, retaddr); > > /* save the current state in the old TSS */ Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c index bab552cd53..3b8fd827e1 100644 --- a/target/i386/tcg/seg_helper.c +++ b/target/i386/tcg/seg_helper.c @@ -378,7 +378,7 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector, /* X86Access avoids memory exceptions during the task switch */ mmu_index = cpu_mmu_index_kernel(env); - access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max, + access_prepare_mmu(&old, env, env->tr.base, old_tss_limit_max + 1, MMU_DATA_STORE, mmu_index, retaddr); if (source == SWITCH_TSS_CALL) { @@ -386,7 +386,8 @@ static int switch_tss_ra(CPUX86State *env, int tss_selector, probe_access(env, tss_base, 2, MMU_DATA_STORE, mmu_index, retaddr); } - access_prepare_mmu(&new, env, tss_base, tss_limit, + /* While true tss_limit may be larger, we don't access the iopb here. */ + access_prepare_mmu(&new, env, tss_base, tss_limit_max + 1, MMU_DATA_LOAD, mmu_index, retaddr); /* save the current state in the old TSS */
The two limit_max variables represent size - 1, just like the encoding in the GDT, thus the 'old' access was off by one. Access the minimal size of the new tss: the complete tss contains the iopb, which may be a larger block than the access api expects, and irrelevant because the iopb is not accessed during the switch itself. Fixes: 8b131065080a ("target/i386/tcg: use X86Access for TSS access") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2511 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/i386/tcg/seg_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)