On Tue, Apr 23, 2024 at 10:15:57AM +0200, Philippe Mathieu-Daudé wrote: > On 22/4/24 21:03, Volker Rümelin wrote: > > Am 20.04.24 um 07:40 schrieb Mark Cave-Ayland: > > > On 20/04/2024 02:21, Richard Henderson wrote: > > > > > > > On 4/19/24 12:51, Mark Cave-Ayland wrote: > > > > > The various Intel CPU manuals claim that SGDT and SIDT can write > > > > > either 24-bits > > > > > or 32-bits depending upon the operand size, but this is incorrect. > > > > > Not only do > > > > > the Intel CPU manuals give contradictory information between processor > > > > > revisions, but this information doesn't even match real-life > > > > > behaviour. > > > > > > > > > > In fact, tests on real hardware show that the CPU always writes > > > > > 32-bits for SGDT > > > > > and SIDT, and this behaviour is required for at least OS/2 Warp and > > > > > WFW 3.11 with > > > > > Win32s to function correctly. Remove the masking applied due to the > > > > > operand size > > > > > for SGDT and SIDT so that the TCG behaviour matches the behaviour on > > > > > real > > > > > hardware. > > > > > > > > > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2198 > > > > > > > > > > -- > > > > > MCA: Whilst I don't have a copy of OS/2 Warp handy, I've confirmed > > > > > that this > > > > > patch fixes the issue in WFW 3.11 with Win32s. For more technical > > > > > information I > > > > > highly recommend the excellent write-up at > > > > > https://www.os2museum.com/wp/sgdtsidt-fiction-and-reality/. > > > > > --- > > > > > target/i386/tcg/translate.c | 14 ++++++++------ > > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c > > > > > index 76a42c679c..3026eb6774 100644 > > > > > --- a/target/i386/tcg/translate.c > > > > > +++ b/target/i386/tcg/translate.c > > > > > @@ -5846,9 +5846,10 @@ static bool disas_insn(DisasContext *s, > > > > > CPUState *cpu) > > > > > gen_op_st_v(s, MO_16, s->T0, s->A0); > > > > > gen_add_A0_im(s, 2); > > > > > tcg_gen_ld_tl(s->T0, tcg_env, offsetof(CPUX86State, > > > > > gdt.base)); > > > > > - if (dflag == MO_16) { > > > > > - tcg_gen_andi_tl(s->T0, s->T0, 0xffffff); > > > > > - } > > > > > + /* > > > > > + * NB: Despite claims to the contrary in Intel CPU > > > > > documentation, > > > > > + * all 32-bits are written regardless of operand > > > > > size. > > > > > + */ > > > > > > > > Current documentation agrees that all 32 bits are written, so I don't > > > > think you need this comment: > > > > > > Ah that's good to know the docs are now correct. I added the comment > > > as there was a lot of conflicting information around for older CPUs so > > > I thought it was worth an explicit mention. > > > > > > If everyone agrees a version without comments is preferable, I can > > > re-send an updated version without them included. > > > > > > > Hi Mark, > > > > I wouldn't remove the comment. > > > > Quote from the Intel® 64 and IA-32 Architectures Software Developer’s > > Manual Volume 2B: Instruction Set Reference, M-U March 2024: > > > > IA-32 Architecture Compatibility > > The 16-bit form of SGDT is compatible with the Intel 286 processor if > > the upper 8 bits are not referenced. The Intel 286 processor fills these > > bits with 1s; processor generations later than the Intel 286 processor > > fill these bits with 0s. > > Is that that OS/2 Warp and WFW 3.11 expect a 286 CPU? QEMU starts > modelling the 486, do we want to consider down to the 286?
Depends on the versions you're talking about. From what I can gather, OS/2 1.x targetted i286, OS/2 2.x & 3.x targetted i386, and OS/2 4.0 Warp targetted i486, while WFW 3.11 was i386. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
