Xen devel - Jun 2005 - [PATCH] Fixing iopl and ioperm

With this patch, x86 and x86-64 share ioport.c, fixing LTP iopl and
ioperm testcase failures (on both). We found an iopl testcase failing
even on x86 xenlinux.

Now x86-64 xenlinux should have the same results regarding the LTP
testcases (as far as we tested).

Signed-off-by: Li B Xin <li.b.xin@intel.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>

Jun
---
Intel Open Source Technology Center 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 11 Jun 2005, at 06:45, Nakajima, Jun wrote:
> With this patch, x86 and x86-64 share ioport.c, fixing LTP iopl and
> ioperm testcase failures (on both). We found an iopl testcase failing
> even on x86 xenlinux.
>
> Now x86-64 xenlinux should have the same results regarding the LTP
> testcases (as far as we tested).
Why does this patch modify the i386 ioport.c so much? I would expect 
that you ought to be able to use the xen/i386 ioport.c with no 
modification at all. The different function prototypes for sys_iopl 
between i386 and x86_64 shouldn''t matter -- x86_64 ought to work with 
teh existing xen/i386 function prototype and function implementation.

The patch you sent is broken in any case -- you cannot find the old 
iopl by looking at eflags, as the real iopl is always zero.

Please re-send with the *smallest possible* changes to 
xen/i386/kernel/ioport.c. If there are justifiable 
cleanups/simplifications, then I''d like to take those as a separate 
patch!

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
> On 11 Jun 2005, at 06:45, Nakajima, Jun wrote:
> 
>> With this patch, x86 and x86-64 share ioport.c, fixing LTP iopl and
>> ioperm testcase failures (on both). We found an iopl testcase
>> failing even on x86 xenlinux. 
>> 
>> Now x86-64 xenlinux should have the same results regarding the LTP
>> testcases (as far as we tested).
> 
> Why does this patch modify the i386 ioport.c so much? I would expect
> that you ought to be able to use the xen/i386 ioport.c with no
> modification at all. The different function prototypes for sys_iopl
> between i386 and x86_64 shouldn''t matter -- x86_64 ought to work
with
> teh existing xen/i386 function prototype and function implementation.
> 
That part is basically coming from the implementation of set_bitmap; x86
and x86-64 uses different implemenations. We''ll try that in a sperate
patch; I think the x86-64 one is simpler.

Well I heard the current x86 (_not_ 64-bit) xenlinux failed one of LTP
iopl/ioperm testcases (and it passes on the native Linux). So I expect
some changes are required there. Maybe that patch should go first. I''ll
double-check it.

Jun
---
Intel Open Source Technology Center

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On xeno32, test case iopl02 of LTP will fail, pls see below:

[root@vt-x bin]# uname -a
Linux vt-x 2.6.11.12-xen0 #1 Mon Jun 13 16:58:09 PDT 2005 i686 i686 i386
GNU/Linux
[root@vt-x bin]# ./iopl02
iopl02      1  PASS  :  Expected failure for Invalid privilege level,
errno: 22
iopl02      2  FAIL  :  Unexpected results for Non super-user ; returned
0 (expected -1), errno 0 (expected errno  1)

With the ioport patch, this bug get fixed.

-Xin

Nakajima, Jun wrote:
> Keir Fraser wrote:
>> On 11 Jun 2005, at 06:45, Nakajima, Jun wrote:
>> 
>>> With this patch, x86 and x86-64 share ioport.c, fixing
>>> LTP iopl and ioperm testcase failures (on both). We
>>> found an iopl testcase failing even on x86 xenlinux. 
>>> 
>>> Now x86-64 xenlinux should have the same results
>>> regarding the LTP testcases (as far as we tested).
>> 
>> Why does this patch modify the i386 ioport.c so much? I
>> would expect that you ought to be able to use the
>> xen/i386 ioport.c with no modification at all. The
>> different function prototypes for sys_iopl between i386
>> and x86_64 shouldn''t matter -- x86_64 ought to work with
>> teh existing xen/i386 function prototype and function
>> implementation.  
>> 
> That part is basically coming from the implementation of
> set_bitmap; x86 and x86-64 uses different implemenations.
> We''ll try that in a sperate patch; I think the x86-64 one
> is simpler. 
> 
> Well I heard the current x86 (_not_ 64-bit) xenlinux
> failed one of LTP iopl/ioperm testcases (and it passes on
> the native Linux). So I expect some changes are required
> there. Maybe that patch should go first. I''ll
> double-check it. 
> 
> Jun
> ---
> Intel Open Source Technology Center
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 14 Jun 2005, at 01:22, Li, Xin B wrote:
> On xeno32, test case iopl02 of LTP will fail, pls see below:
>
My guess is that LTP tries to change iopl to 1. This doesn;t fail on 
xenlinux because the iopl is *already* 1 (because the kernel runs at 
ring 1; not ring 0 as on native). If this is the case, your patch 
''fixes'' LTP by breaking our implementation of sys_iopl. By
reading the
old iopl out of eflags, you will always read zero. This would certainly 
fix this LTP testcase but it is the wrong thing to do.

Frankly, fixing this LTP test case will be a pain. If I have correctly 
described what''s going on then let''s ignore it at least for
now -- it
is not indicative of a real bug and it is entirely harmless. LTP is a 
measure of stability for us, but passing every LTP test is not an end 
in itself (we should understand the failures though).

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 14 Jun 2005, at 14:21, Keir Fraser wrote:
> Frankly, fixing this LTP test case will be a pain. If I have correctly 
> described what''s going on then let''s ignore it at least
for now -- it
> is not indicative of a real bug and it is entirely harmless. LTP is a 
> measure of stability for us, but passing every LTP test is not an end 
> in itself (we should understand the failures though).
Actually this is perhaps worth fixing, just to prevent a super-user 
process from denying I/O access to the kernel itself (by running 
iopl(0)). But please ignore the test failure in your patch: I can fix 
it properly next week.

Also, please don''t change set_bitmap() unless you are fixing a bug to 
make it work with x86_64. I don;t care about simplifying its 
implementation -- I''d rather keep the diff against the native i386 
implementation as small as possible. As I originally said, I wonder if 
you actually need any changes to xen/i386/kernel/ioport.c at all.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel