Xen devel - Nov 2013 - [xen-unstable test] 21486: tolerable FAIL - PUSHED

flight 21486 xen-unstable real [real]
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pcipt-intel  9 guest-start                 fail never pass
 test-armhf-armhf-xl           5 xen-boot                     fail   never pass
 test-amd64-i386-xend-winxpsp3 16 leak-check/check             fail  never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 13 guest-stop             fail never pass
 test-amd64-amd64-xl-qemuu-winxpsp3 13 guest-stop               fail never pass
 test-amd64-i386-xl-winxpsp3-vcpus1 13 guest-stop               fail never pass
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 13 guest-stop         fail never pass
 test-amd64-i386-xend-qemut-winxpsp3 16 leak-check/check        fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 13 guest-stop             fail never pass
 test-amd64-amd64-xl-qemut-winxpsp3 13 guest-stop               fail never pass
 test-amd64-amd64-xl-winxpsp3 13 guest-stop                   fail   never pass
 test-amd64-amd64-xl-win7-amd64 13 guest-stop                   fail never pass
 test-amd64-i386-xl-win7-amd64 13 guest-stop                   fail  never pass
 test-amd64-i386-xl-qemut-win7-amd64 13 guest-stop              fail never pass

version targeted for testing:
 xen                  6f366e276264f61b752d9eea63c42021b8fffec6
baseline version:
 xen                  68bd172e6fa565899c846eb72755c8ffd8562c8a

------------------------------------------------------------
People who touched revisions under test:
  Andrew Cooper <andrew.cooper3@citrix.com>
  Don Slutz <dslutz@verizon.com>
  Ian Campbell <ian.campbell@citrix.com>
  Ian Jackson <Ian.Jackson@eu.citrix.com>
  Jan Beulich <jbeulich@suse.com>
  Julien Grall <julien.grall@linaro.org>
  Keir Fraser <keir@xen.org>
  Roger Pau Monné <roger.pau@citrix.com>
------------------------------------------------------------

jobs:
 build-amd64                                                  pass    
 build-armhf                                                  pass    
 build-i386                                                   pass    
 build-amd64-oldkern                                          pass    
 build-i386-oldkern                                           pass    
 build-amd64-pvops                                            pass    
 build-armhf-pvops                                            pass    
 build-i386-pvops                                             pass    
 test-amd64-amd64-xl                                          pass    
 test-armhf-armhf-xl                                          fail    
 test-amd64-i386-xl                                           pass    
 test-amd64-i386-rhel6hvm-amd                                 pass    
 test-amd64-i386-qemut-rhel6hvm-amd                           pass    
 test-amd64-i386-qemuu-rhel6hvm-amd                           pass    
 test-amd64-amd64-xl-qemut-win7-amd64                         fail    
 test-amd64-i386-xl-qemut-win7-amd64                          fail    
 test-amd64-amd64-xl-qemuu-win7-amd64                         fail    
 test-amd64-amd64-xl-win7-amd64                               fail    
 test-amd64-i386-xl-win7-amd64                                fail    
 test-amd64-i386-xl-credit2                                   pass    
 test-amd64-amd64-xl-pcipt-intel                              fail    
 test-amd64-i386-rhel6hvm-intel                               pass    
 test-amd64-i386-qemut-rhel6hvm-intel                         pass    
 test-amd64-i386-qemuu-rhel6hvm-intel                         pass    
 test-amd64-i386-xl-multivcpu                                 pass    
 test-amd64-amd64-pair                                        pass    
 test-amd64-i386-pair                                         pass    
 test-amd64-amd64-xl-sedf-pin                                 pass    
 test-amd64-amd64-pv                                          pass    
 test-amd64-i386-pv                                           pass    
 test-amd64-amd64-xl-sedf                                     pass    
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1                     fail    
 test-amd64-i386-xl-winxpsp3-vcpus1                           fail    
 test-amd64-i386-xend-qemut-winxpsp3                          fail    
 test-amd64-amd64-xl-qemut-winxpsp3                           fail    
 test-amd64-amd64-xl-qemuu-winxpsp3                           fail    
 test-amd64-i386-xend-winxpsp3                                fail    
 test-amd64-amd64-xl-winxpsp3                                 fail    


------------------------------------------------------------
sg-report-flight on woking.cam.xci-test.com
logs: /home/xc_osstest/logs
images: /home/xc_osstest/images

Logs, config files, etc. are available at
    http://www.chiark.greenend.org.uk/~xensrcts/logs

Test harness code can be found at
    http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=xen-unstable
+ revision=6f366e276264f61b752d9eea63c42021b8fffec6
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getconfig Repos
+++ perl -e ''
                use Osstest;
                readglobalconfig();
                print $c{"Repos"} or die $!;
        ''
++ repos=/export/home/osstest/repos
++ repos_lock=/export/home/osstest/repos/lock
++ ''['' x ''!=''
x/export/home/osstest/repos/lock '']''
++ OSSTEST_REPOS_LOCK_LOCKED=/export/home/osstest/repos/lock
++ exec with-lock-ex -w /export/home/osstest/repos/lock ./ap-push xen-unstable
6f366e276264f61b752d9eea63c42021b8fffec6
+ branch=xen-unstable
+ revision=6f366e276264f61b752d9eea63c42021b8fffec6
+ . cri-lock-repos
++ . cri-common
+++ . cri-getconfig
+++ umask 002
+++ getconfig Repos
+++ perl -e ''
                use Osstest;
                readglobalconfig();
                print $c{"Repos"} or die $!;
        ''
++ repos=/export/home/osstest/repos
++ repos_lock=/export/home/osstest/repos/lock
++ ''['' x/export/home/osstest/repos/lock ''!=''
x/export/home/osstest/repos/lock '']''
+ . cri-common
++ . cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable
+ ''['' xxen = xlinux '']''
+ linuxbranch+ : tested/2.6.39.x
+ . ap-common
++ : osstest@xenbits.xensource.com
++ : git://xenbits.xen.org/xen.git
++ : osstest@xenbits.xensource.com:/home/xen/git/xen.git
++ : git://xenbits.xen.org/staging/qemu-xen-unstable.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osstest@xenbits.xensource.com:/home/osstest/ext/linux-firmware.git
++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
++ : osstest@xenbits.xensource.com:/home/xen/git/linux-pvops.git
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-3.4
++ : tested/linux-arm-xen
++ ''['' xgit://xenbits.xen.org/linux-pvops.git = x
'']''
++ ''['' x = x '']''
++ : git://xenbits.xen.org/linux-pvops.git
++ : tested/linux-arm-xen
++ : git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git
++ : tested/2.6.39.x
++ : daily-cron.xen-unstable
++ : daily-cron.xen-unstable
++ : http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27
++ : git://xenbits.xen.org/staging/qemu-upstream-unstable.git
++ : daily-cron.xen-unstable
+ TREE_LINUX=osstest@xenbits.xensource.com:/home/xen/git/linux-pvops.git
+
TREE_QEMU_UPSTREAM=osstest@xenbits.xensource.com:/home/xen/git/qemu-upstream-unstable.git
+ TREE_XEN=osstest@xenbits.xensource.com:/home/xen/git/xen.git
+ info_linux_tree xen-unstable
+ case $1 in
+ return 1
+ case "$branch" in
+ cd /export/home/osstest/repos/xen
+ git push osstest@xenbits.xensource.com:/home/xen/git/xen.git
6f366e276264f61b752d9eea63c42021b8fffec6:master
Total 0 (delta 0), reused 0 (delta 0)
To osstest@xenbits.xensource.com:/home/xen/git/xen.git
   68bd172..6f366e2  6f366e276264f61b752d9eea63c42021b8fffec6 -> master


--===============0804208837068111303=Content-Type: text/plain;
charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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

--===============0804208837068111303==--

On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
> flight 21486 xen-unstable real [real]
> http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
[...]
>  test-armhf-armhf-xl           5 xen-boot                     fail   never
pass
Switching the ARM tests over to Linux 3.12 means we now get much further
and run into:

(from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
> [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank gpio_request 0
failed: -517
> [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci ffe08000.sata: AHCI
0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
> [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci ffe08000.sata:
flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc apst
> [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 : sata_highbank
> [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 : sata_highbank
> [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 : sata_highbank
> [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 : sata_highbank
> [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 : sata_highbank
> [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max UDMA/133 mmio [mem
0xffe08000-0xffe17fff] port 0x100 irq 115
> [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max UDMA/133 mmio [mem
0xffe08000-0xffe17fff] port 0x180 irq 115
> [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max UDMA/133 mmio [mem
0xffe08000-0xffe17fff] port 0x200 irq 115
> [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max UDMA/133 mmio [mem
0xffe08000-0xffe17fff] port 0x280 irq 115
[...]
> [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max UDMA/133 mmio [mem
0xffe08000-0xffe17fff] port 0x300 irq 115
> [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset failed (1st FIS
failed)
> [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset failed (1st FIS
failed)
> [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset failed (1st FIS
failed)
> [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting SATA link speed to
1.5 Gbps
> [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset failed (1st FIS
failed)
> [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset failed, giving up
> [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link down (SStatus 0
SControl 300)
> [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link down (SStatus 0
SControl 300)
> [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link down (SStatus 0
SControl 300)
> [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link down (SStatus 0
SControl 300)
[...]
> [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system ... Begin:
Running /scripts/local-top ...   Volume group "marilith-n5" not found
... fail to find root.

Has anyone seen this before, is it a known 3.12 on Midway issue perhaps?

I think the failure is too soon to be related a lack of swiotlb? But
maybe we should enable the 1:1 workaround for Midway for now anyway?

Or perhaps we should make it the default in the absence of SMMU?
Stefano, how did you end up coordinating this stuff with the dom0 kernel
side of things?

Ian.

On Wed, 6 Nov 2013, Ian Campbell wrote:
> On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
> 
> > flight 21486 xen-unstable real [real]
> > http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
> [...]
> >  test-armhf-armhf-xl           5 xen-boot                     fail  
never pass
> 
> Switching the ARM tests over to Linux 3.12 means we now get much further
> and run into:
> 
> (from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
> 
> > [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank gpio_request 0
failed: -517
> > [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci ffe08000.sata:
AHCI 0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
> > [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci ffe08000.sata:
flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc apst
> > [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 : sata_highbank
> > [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 : sata_highbank
> > [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 : sata_highbank
> > [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 : sata_highbank
> > [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 : sata_highbank
> > [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x100 irq 115
> > [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x180 irq 115
> > [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x200 irq 115
> > [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x280 irq 115
> [...]
> > [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x300 irq 115
> > [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset failed (1st
FIS failed)
> > [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset failed (1st
FIS failed)
> > [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset failed (1st
FIS failed)
> > [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting SATA link
speed to 1.5 Gbps
> > [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset failed (1st
FIS failed)
> > [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset failed, giving up
> > [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link down (SStatus
0 SControl 300)
> > [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link down (SStatus
0 SControl 300)
> > [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link down (SStatus
0 SControl 300)
> > [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link down (SStatus
0 SControl 300)
> [...]
> > [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system ... Begin:
Running /scripts/local-top ...   Volume group "marilith-n5" not found
> ... fail to find root.
> 
> Has anyone seen this before, is it a known 3.12 on Midway issue perhaps?
> 
> I think the failure is too soon to be related a lack of swiotlb?
No, I think it''s probably it.

> But maybe we should enable the 1:1 workaround for Midway for now anyway?
We definitely need one or the other.

> Or perhaps we should make it the default in the absence of SMMU?
> Stefano, how did you end up coordinating this stuff with the dom0 kernel
> side of things?
I am assuming the presence of the 1:1 mapping and then only dealing with
foreign grants in Linux. So yes, I think that we should just make the
1:1 workaround the default until we get SMMU support.

On Wed, 2013-11-06 at 11:14 +0000, Stefano Stabellini
wrote:
> On Wed, 6 Nov 2013, Ian Campbell wrote:
> > On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
> > 
> > > flight 21486 xen-unstable real [real]
> > > http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
> > [...]
> > >  test-armhf-armhf-xl           5 xen-boot                    
fail   never pass
> > 
> > Switching the ARM tests over to Linux 3.12 means we now get much
further
> > and run into:
> > 
> > (from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
> > 
> > > [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank
gpio_request 0 failed: -517
> > > [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci
ffe08000.sata: AHCI 0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
> > > [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci
ffe08000.sata: flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc
apst
> > > [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 : sata_highbank
> > > [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 : sata_highbank
> > > [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 : sata_highbank
> > > [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 : sata_highbank
> > > [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 : sata_highbank
> > > [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max UDMA/133
mmio [mem 0xffe08000-0xffe17fff] port 0x100 irq 115
> > > [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max UDMA/133
mmio [mem 0xffe08000-0xffe17fff] port 0x180 irq 115
> > > [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max UDMA/133
mmio [mem 0xffe08000-0xffe17fff] port 0x200 irq 115
> > > [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max UDMA/133
mmio [mem 0xffe08000-0xffe17fff] port 0x280 irq 115
> > [...]
> > > [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max UDMA/133
mmio [mem 0xffe08000-0xffe17fff] port 0x300 irq 115
> > > [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset failed
(1st FIS failed)
> > > [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset failed
(1st FIS failed)
> > > [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset failed
(1st FIS failed)
> > > [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting SATA link
speed to 1.5 Gbps
> > > [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset failed
(1st FIS failed)
> > > [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset failed,
giving up
> > > [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link down
(SStatus 0 SControl 300)
> > > [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link down
(SStatus 0 SControl 300)
> > > [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link down
(SStatus 0 SControl 300)
> > > [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link down
(SStatus 0 SControl 300)
> > [...]
> > > [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system ...
Begin: Running /scripts/local-top ...   Volume group "marilith-n5" not
found
> > ... fail to find root.
> > 
> > Has anyone seen this before, is it a known 3.12 on Midway issue
perhaps?
> > 
> > I think the failure is too soon to be related a lack of swiotlb?
> 
> No, I think it''s probably it.
OK.
> > But maybe we should enable the 1:1 workaround for Midway for now
anyway?
> 
> We definitely need one or the other.
Ack.
> > Or perhaps we should make it the default in the absence of SMMU?
> > Stefano, how did you end up coordinating this stuff with the dom0
kernel
> > side of things?
> 
> I am assuming the presence of the 1:1 mapping and then only dealing with
> foreign grants in Linux. So yes, I think that we should just make the
> 1:1 workaround the default until we get SMMU support.
Do you have a patch to that affect?

BTW, at one point we discussed relaxing the 1:1 mapping into just a
contiguous mapping and publishing the offset to the guest, whatever
became of that idea?

Julien solved his issue on Arndale by allocating the 1:1 from as close
to the start of physical RAM as he could, but having an offset would be
more elegant and less prone to spurious breakage I think.

Ian.

On Wed, 6 Nov 2013, Ian Campbell wrote:
> On Wed, 2013-11-06 at 11:14 +0000, Stefano Stabellini wrote:
> > On Wed, 6 Nov 2013, Ian Campbell wrote:
> > > On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
> > > 
> > > > flight 21486 xen-unstable real [real]
> > > > http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
> > > [...]
> > > >  test-armhf-armhf-xl           5 xen-boot                   
fail   never pass
> > > 
> > > Switching the ARM tests over to Linux 3.12 means we now get much
further
> > > and run into:
> > > 
> > > (from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
> > > 
> > > > [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank
gpio_request 0 failed: -517
> > > > [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci
ffe08000.sata: AHCI 0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
> > > > [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci
ffe08000.sata: flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc
apst
> > > > [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 :
sata_highbank
> > > > [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 :
sata_highbank
> > > > [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 :
sata_highbank
> > > > [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 :
sata_highbank
> > > > [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 :
sata_highbank
> > > > [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x100 irq 115
> > > > [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x180 irq 115
> > > > [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x200 irq 115
> > > > [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x280 irq 115
> > > [...]
> > > > [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x300 irq 115
> > > > [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset
failed (1st FIS failed)
> > > > [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset
failed (1st FIS failed)
> > > > [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset
failed (1st FIS failed)
> > > > [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting SATA
link speed to 1.5 Gbps
> > > > [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset
failed (1st FIS failed)
> > > > [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset failed,
giving up
> > > > [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link
down (SStatus 0 SControl 300)
> > > > [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link
down (SStatus 0 SControl 300)
> > > > [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link
down (SStatus 0 SControl 300)
> > > > [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link
down (SStatus 0 SControl 300)
> > > [...]
> > > > [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system
... Begin: Running /scripts/local-top ...   Volume group "marilith-n5"
not found
> > > ... fail to find root.
> > > 
> > > Has anyone seen this before, is it a known 3.12 on Midway issue
perhaps?
> > > 
> > > I think the failure is too soon to be related a lack of swiotlb?
> > 
> > No, I think it''s probably it.
> 
> OK.
> 
> > > But maybe we should enable the 1:1 workaround for Midway for now
anyway?
> > 
> > We definitely need one or the other.
> 
> Ack.
> 
> > > Or perhaps we should make it the default in the absence of SMMU?
> > > Stefano, how did you end up coordinating this stuff with the dom0
kernel
> > > side of things?
> > 
> > I am assuming the presence of the 1:1 mapping and then only dealing
with
> > foreign grants in Linux. So yes, I think that we should just make the
> > 1:1 workaround the default until we get SMMU support.
> 
> Do you have a patch to that affect?
Just sent. I have more patches in my queue, most of them by Julien and
Andre and might need to be reworked. In particular the patch to
introduce SMP support.

> BTW, at one point we discussed relaxing the 1:1 mapping into just a
> contiguous mapping and publishing the offset to the guest, whatever
> became of that idea?
> 
> Julien solved his issue on Arndale by allocating the 1:1 from as close
> to the start of physical RAM as he could, but having an offset would be
> more elegant and less prone to spurious breakage I think.
I take that by "and publishing the offset to the guest" you mean
adjusting the start of the memory region in the device tree?

On 11/06/2013 08:40 PM, Stefano Stabellini wrote:
> On Wed, 6 Nov 2013, Ian Campbell wrote:
>> On Wed, 2013-11-06 at 11:14 +0000, Stefano Stabellini wrote:
>>> On Wed, 6 Nov 2013, Ian Campbell wrote:
>>>> On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
>>>>
>>>>> flight 21486 xen-unstable real [real]
>>>>> http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
>>>> [...]
>>>>>   test-armhf-armhf-xl           5 xen-boot                 
fail   never pass
>>>>
>>>> Switching the ARM tests over to Linux 3.12 means we now get
much further
>>>> and run into:
>>>>
>>>> (from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
>>>>
>>>>> [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank
gpio_request 0 failed: -517
>>>>> [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci
ffe08000.sata: AHCI 0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
>>>>> [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci
ffe08000.sata: flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc
apst
>>>>> [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 :
sata_highbank
>>>>> [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 :
sata_highbank
>>>>> [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 :
sata_highbank
>>>>> [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 :
sata_highbank
>>>>> [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 :
sata_highbank
>>>>> [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x100 irq 115
>>>>> [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x180 irq 115
>>>>> [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x200 irq 115
>>>>> [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x280 irq 115
>>>> [...]
>>>>> [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max
UDMA/133 mmio [mem 0xffe08000-0xffe17fff] port 0x300 irq 115
>>>>> [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset
failed (1st FIS failed)
>>>>> [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset
failed (1st FIS failed)
>>>>> [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset
failed (1st FIS failed)
>>>>> [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting
SATA link speed to 1.5 Gbps
>>>>> [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset
failed (1st FIS failed)
>>>>> [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset
failed, giving up
>>>>> [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link
down (SStatus 0 SControl 300)
>>>>> [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link
down (SStatus 0 SControl 300)
>>>>> [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link
down (SStatus 0 SControl 300)
>>>>> [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link
down (SStatus 0 SControl 300)
>>>> [...]
>>>>> [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system
... Begin: Running /scripts/local-top ...   Volume group "marilith-n5"
not found
>>>> ... fail to find root.
>>>>
>>>> Has anyone seen this before, is it a known 3.12 on Midway issue
perhaps?
>>>>
>>>> I think the failure is too soon to be related a lack of
swiotlb?
>>>
>>> No, I think it''s probably it.
>>
>> OK.
>>
>>>> But maybe we should enable the 1:1 workaround for Midway for
now anyway?
>>>
>>> We definitely need one or the other.
>>
>> Ack.
>>
>>>> Or perhaps we should make it the default in the absence of
SMMU?
>>>> Stefano, how did you end up coordinating this stuff with the
dom0 kernel
>>>> side of things?
>>>
>>> I am assuming the presence of the 1:1 mapping and then only dealing
with
>>> foreign grants in Linux. So yes, I think that we should just make
the
>>> 1:1 workaround the default until we get SMMU support.
>>
>> Do you have a patch to that affect?
>
> Just sent. I have more patches in my queue, most of them by Julien and
> Andre and might need to be reworked. In particular the patch to
> introduce SMP support.
Yes, Julien and I agreed last week that he will send the remaining fixes 
out ASAP.
So beside that one above (thanks for sending, Stefano!), we have the SMP 
patch, Julien''s "Dom0 interrupt" series and some minor
things. Should be
all in Julien''s tree on xenbits.

Regards,
Andre.
>> BTW, at one point we discussed relaxing the 1:1 mapping into just a
>> contiguous mapping and publishing the offset to the guest, whatever
>> became of that idea?
>>
>> Julien solved his issue on Arndale by allocating the 1:1 from as close
>> to the start of physical RAM as he could, but having an offset would be
>> more elegant and less prone to spurious breakage I think.
>
> I take that by "and publishing the offset to the guest" you mean
> adjusting the start of the memory region in the device tree?
>

On Wed, 2013-11-06 at 19:40 +0000, Stefano Stabellini
wrote:
> > > > Or perhaps we should make it the default in the absence of
SMMU?
> > > > Stefano, how did you end up coordinating this stuff with the
dom0 kernel
> > > > side of things?
> > > 
> > > I am assuming the presence of the 1:1 mapping and then only
dealing with
> > > foreign grants in Linux. So yes, I think that we should just make
the
> > > 1:1 workaround the default until we get SMMU support.
> > 
> > Do you have a patch to that affect?
> 
> Just sent.
Got it. I actually meant a patch making it the default globally, but
this one will do for now.
>  I have more patches in my queue, most of them by Julien and
> Andre and might need to be reworked. In particular the patch to
> introduce SMP support.
> 
> 
> > BTW, at one point we discussed relaxing the 1:1 mapping into just a
> > contiguous mapping and publishing the offset to the guest, whatever
> > became of that idea?
> > 
> > Julien solved his issue on Arndale by allocating the 1:1 from as close
> > to the start of physical RAM as he could, but having an offset would
be
> > more elegant and less prone to spurious breakage I think.
> 
> I take that by "and publishing the offset to the guest" you mean
> adjusting the start of the memory region in the device tree?
No, that''s what we do today. i.e. if RAM is at 0x80000000 and we
allocate a 1:1 region at 0x80800000 then we give the guest a RAM region
at 0x80800000. This caused breakage on Exynos, so Julien change things
(6c21cb36e263 "Allocate memory for dom0 from the bottom with the 1:1
Workaround") to increase the chances that the memory would end up
allocated from closer to 0x80000000, which mostly works because Xen
normally tends to allocate the highest address it can which meets the
callers constraints (bit width etc) and we allocate memory for dom0
fairly early. But it is potentially fragile.

What I''m suggesting is to allocate a contiguous block of memory from
wherever (e.g 0x80800000) but tell the guest it has RAM at 0x80000000
(and map it there in the p2m) and separately expose the 0x00800000
offset for use in the various phys_to_bus calculations (in the swiotlb
or wherever). IOW in the places which your series determines it is safe
to just return the pseudophysical address it would instead apply the
offset.

I''m thinking in terms of a property in the hypervisor node, e.g.
dma-offset = <0x00800000>;

Obviously we would omit this (or explicitly set it to zero) when SMMU is
present.  

How are we intending to signal to dom0 that it needn''t do all the
swiotlb magic even for foreign pages when an SMMU is present? We should
build that scheme in now. Presence/absence of dma-offset might be a
convenient hook, but maybe it needs to be per-device?

Ian.

On Thu, 7 Nov 2013, Ian Campbell wrote:
> > > BTW, at one point we discussed relaxing the 1:1 mapping into just
a
> > > contiguous mapping and publishing the offset to the guest,
whatever
> > > became of that idea?
> > > 
> > > Julien solved his issue on Arndale by allocating the 1:1 from as
close
> > > to the start of physical RAM as he could, but having an offset
would be
> > > more elegant and less prone to spurious breakage I think.
> > 
> > I take that by "and publishing the offset to the guest" you
mean
> > adjusting the start of the memory region in the device tree?
> 
> No, that''s what we do today. i.e. if RAM is at 0x80000000 and we
> allocate a 1:1 region at 0x80800000 then we give the guest a RAM region
> at 0x80800000. This caused breakage on Exynos, so Julien change things
> (6c21cb36e263 "Allocate memory for dom0 from the bottom with the 1:1
> Workaround") to increase the chances that the memory would end up
> allocated from closer to 0x80000000, which mostly works because Xen
> normally tends to allocate the highest address it can which meets the
> callers constraints (bit width etc) and we allocate memory for dom0
> fairly early. But it is potentially fragile.
> 
> What I''m suggesting is to allocate a contiguous block of memory
from
> wherever (e.g 0x80800000) but tell the guest it has RAM at 0x80000000
> (and map it there in the p2m) and separately expose the 0x00800000
> offset for use in the various phys_to_bus calculations (in the swiotlb
> or wherever). IOW in the places which your series determines it is safe
> to just return the pseudophysical address it would instead apply the
> offset.
> 
> I''m thinking in terms of a property in the hypervisor node, e.g.
> dma-offset = <0x00800000>;
> 
> Obviously we would omit this (or explicitly set it to zero) when SMMU is
> present.  
Technically it would work, but it has the feeling of an ad-hoc hack.

Basically we are doing this to make it look like the RAM is starting in
the same position as the native platform and at the same time have a
bit more freedom in dom0 memory allocation in the hypervisor.

However why do we even have a device tree property to express where the
memory start, when this property can only have a single value?

I think that Xen should be free to make the guest memory start at any
address it can express in device tree, within reasonable limits.
Otherwise we can go back to the world of add-hoc hardcoded constants and
get rid of device tree entirely.
If this doesn''t work, why the kernel can''t be fixed?
Am I being unreasonable about this?

> How are we intending to signal to dom0 that it needn''t do all the
> swiotlb magic even for foreign pages when an SMMU is present? We should
> build that scheme in now. Presence/absence of dma-offset might be a
> convenient hook, but maybe it needs to be per-device?
 
There there cases:

1) no IOMMU
2) all the devices are behind an IOMMU
3) some devices are behind an IOMMU

1) and 2) are easy and a global property under the Xen node would
suffice.
3) is the difficult case. We would need to express that some devices
need to go through swiotlb-xen while others don''t.
We could add a xen-dma-safe property under each dma capable
device that is behind an IOMMU programmed by Xen.
It might be best to wait for IOMMU support in the hypervisor before we
get into this.

On Mon, 2013-11-11 at 18:42 +0000, Stefano Stabellini
wrote:
> On Thu, 7 Nov 2013, Ian Campbell wrote:
> > > > BTW, at one point we discussed relaxing the 1:1 mapping into
just a
> > > > contiguous mapping and publishing the offset to the guest,
whatever
> > > > became of that idea?
> > > > 
> > > > Julien solved his issue on Arndale by allocating the 1:1
from as close
> > > > to the start of physical RAM as he could, but having an
offset would be
> > > > more elegant and less prone to spurious breakage I think.
> > > 
> > > I take that by "and publishing the offset to the guest"
you mean
> > > adjusting the start of the memory region in the device tree?
> > 
> > No, that''s what we do today. i.e. if RAM is at 0x80000000 and
we
> > allocate a 1:1 region at 0x80800000 then we give the guest a RAM
region
> > at 0x80800000. This caused breakage on Exynos, so Julien change things
> > (6c21cb36e263 "Allocate memory for dom0 from the bottom with the
1:1
> > Workaround") to increase the chances that the memory would end up
> > allocated from closer to 0x80000000, which mostly works because Xen
> > normally tends to allocate the highest address it can which meets the
> > callers constraints (bit width etc) and we allocate memory for dom0
> > fairly early. But it is potentially fragile.
> > 
> > What I''m suggesting is to allocate a contiguous block of
memory from
> > wherever (e.g 0x80800000) but tell the guest it has RAM at 0x80000000
> > (and map it there in the p2m) and separately expose the 0x00800000
> > offset for use in the various phys_to_bus calculations (in the swiotlb
> > or wherever). IOW in the places which your series determines it is
safe
> > to just return the pseudophysical address it would instead apply the
> > offset.
> > 
> > I''m thinking in terms of a property in the hypervisor node,
e.g.
> > dma-offset = <0x00800000>;
> > 
> > Obviously we would omit this (or explicitly set it to zero) when SMMU
is
> > present.  
> 
> Technically it would work, but it has the feeling of an ad-hoc hack.
> 
> Basically we are doing this to make it look like the RAM is starting in
> the same position as the native platform and at the same time have a
> bit more freedom in dom0 memory allocation in the hypervisor.
Correct.
> However why do we even have a device tree property to express where the
> memory start, when this property can only have a single value?
> 
> I think that Xen should be free to make the guest memory start at any
> address it can express in device tree, within reasonable limits.
> Otherwise we can go back to the world of add-hoc hardcoded constants and
> get rid of device tree entirely.
> If this doesn''t work, why the kernel can''t be fixed?
> Am I being unreasonable about this?
Unfortunately the reality is that the Linux kernel has various
constraints on where RAM might be and how it is aligned WRT to the
kernel load address. This is particularly bad for non-multiplatform
kernels (i.e. Julien had issues on Arndale) but there are aso some
constraints on non-multiplatform ones (I don''t think we''ve
tripped over
any of them).

We could fight this and try and fix it but it would basically be a case
of "Xen dom0 is special compared to the real hardware", and it could
involve some nasty start-of-day munging.

Overall a conceptually simple and well defined "ad-hoc hack" seems
preferable to me.
> > How are we intending to signal to dom0 that it needn''t do all
the
> > swiotlb magic even for foreign pages when an SMMU is present? We
should
> > build that scheme in now. Presence/absence of dma-offset might be a
> > convenient hook, but maybe it needs to be per-device?
>  
> There there cases:
> 
> 1) no IOMMU
> 2) all the devices are behind an IOMMU
> 3) some devices are behind an IOMMU
> 
> 1) and 2) are easy and a global property under the Xen node would
> suffice.
> 3) is the difficult case. We would need to express that some devices
> need to go through swiotlb-xen while others don''t.
> We could add a xen-dma-safe property under each dma capable
> device that is behind an IOMMU programmed by Xen.
> It might be best to wait for IOMMU support in the hypervisor before we
> get into this.
Sure, I didn''t want to suggest otherwise, just that whatever schemes we
put in place today were designed to be forwards compatible. "build"
was
probably a bad choice of words, "think about" might have been better.

Ian.

Arnd, Olof,
we have been having this discussion on xen-devel regarding whether Xen
should be allowed to modify the start address of the physical memory
region in device tree before passing it to dom0 or not.

The reason why this question is coming up now, is that we realized that
we are going to have to live with the 1:1 pseudo-physical to physical
mapping for dom0 for a while. This limits the ability of the hypervisor
of allocating dom0 memory wherever it wants. Xen can allocate dom0
memory from the low end but maybe not exactly from the start.

As a result we would adjust the start of physical memory in device tree
to match the start of the memory region allocated for dom0. For example
on the Arndale it could be 0x80800000 instead of 0x80000000.

Unfortunately not all the platforms can cope with this very well. In
particular the Arndale seems to have issues.


The question for you, as arm-soc maintainers, is: do you think this
should work and if we find any issues we should just fix them or report
them as bugs?
Is this entirely going away with multiplatform kernels so we shouldn''t
worry about it?
Or is this a lost fight and should we find a workaround (see below if we
are curious) to make the start of memory look the same?




On Tue, 12 Nov 2013, Ian Campbell wrote:
> > However why do we even have a device tree property to express where
the
> > memory start, when this property can only have a single value?
> > 
> > I think that Xen should be free to make the guest memory start at any
> > address it can express in device tree, within reasonable limits.
> > Otherwise we can go back to the world of add-hoc hardcoded constants
and
> > get rid of device tree entirely.
> > If this doesn''t work, why the kernel can''t be fixed?
> > Am I being unreasonable about this?
> 
> Unfortunately the reality is that the Linux kernel has various
> constraints on where RAM might be and how it is aligned WRT to the
> kernel load address. This is particularly bad for non-multiplatform
> kernels (i.e. Julien had issues on Arndale) but there are aso some
> constraints on non-multiplatform ones (I don''t think
we''ve tripped over
> any of them).
> 
> We could fight this and try and fix it but it would basically be a case
> of "Xen dom0 is special compared to the real hardware", and it
could
> involve some nasty start-of-day munging.
> 
> Overall a conceptually simple and well defined "ad-hoc hack"
seems
> preferable to me.

On Tue, Nov 12, 2013 at 12:25:18PM +0000, Stefano Stabellini
wrote:
> Arnd, Olof,
This isn''t really an arm-soc thing, it''s a core ARM thing...
> we have been having this discussion on xen-devel regarding whether Xen
> should be allowed to modify the start address of the physical memory
> region in device tree before passing it to dom0 or not.
> 
> The reason why this question is coming up now, is that we realized that
> we are going to have to live with the 1:1 pseudo-physical to physical
> mapping for dom0 for a while. This limits the ability of the hypervisor
> of allocating dom0 memory wherever it wants. Xen can allocate dom0
> memory from the low end but maybe not exactly from the start.
> 
> As a result we would adjust the start of physical memory in device tree
> to match the start of the memory region allocated for dom0. For example
> on the Arndale it could be 0x80800000 instead of 0x80000000.
> 
> Unfortunately not all the platforms can cope with this very well. In
> particular the Arndale seems to have issues.
That should be no problem provided that:

(a) you load the kernel somewhere between 0x80800000 and 0x80ffffff -
the decompressor will decide that the start of memory is 0x80800000, and
place the kernel at 0x80808000.
(b) _at the moment_ you modify DT to specify that memory starts at
0x80800000 and not 0x80000000.

(b) is going to change soon: the shmobile and zynq platforms already have
a problem with their memory setup which needs a patch in this area, and
the patch will have the side effect of automatically removing (in your
case) 0x80000000 to 0x80800000.  See the patch below.

If there''s any other issues with multiplatform, then yes, we want to
hear
about them.

 arch/arm/kernel/setup.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index f52150d2ec00..1957d54198ad 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size)
 	}
 #endif
 
+	if (aligned_start < PHYS_OFFSET) {
+		if (aligned_start + size < PHYS_OFFSET) {
+			pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n",
+				aligned_start, aligned_start + size);
+			return -EINVAL;
+		}
+
+		size -= PHYS_OFFSET - aligned_start;
+		aligned_start = PHYS_OFFSET;
+	}
+
 	bank->start = aligned_start;
 	bank->size = size & ~(phys_addr_t)(PAGE_SIZE - 1);

On Tuesday 12 November 2013, Stefano Stabellini wrote:

Hi Stefano,

I haven''t given it too much thought, but here is what I believe should
be done:
> The question for you, as arm-soc maintainers, is: do you think this
> should work and if we find any issues we should just fix them or report
> them as bugs?
Modifying the DT to mark anything as "reserved" or absent that Dom0
should or can not touch sounds like the correct way to do this. Whether
this needs to be done by modifying the reg property of the device node
or through a different method I can''t tell.

If you find bugs in the kernel that prevent this from working, but it
works fine for everyone else, it''s up to you to provide a bug-fix,
which would most likely be up to Russell to apply.
> Is this entirely going away with multiplatform kernels so we
shouldn''t
> worry about it?
Multiplatform kernels are by definition relocatable using
CONFIG_ARM_PATCH_PHYS_VIRT, within some limitations such as the
granularity of the mapping. You certainly can''t move the start of
memory
to an address of smaller than 2MB (hugepage) alignment, but you might
need something larger than that.
> Or is this a lost fight and should we find a workaround (see below if we
> are curious) to make the start of memory look the same?
I don''t see what hack you are referring to, can you elaborate?

My feeling is that we should maintain the requirement that that it must be
possible to enable Dom0 support on any virtualisation-capable platform
without breaking other platforms or causing an unreasonable run-time
overhead.

BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
non-LPAE ARMv6/v7 multiplatform build?

	Arnd

On 11/06/2013 09:52 AM, Ian Campbell wrote:
> On Wed, 2013-11-06 at 06:52 +0000, xen.org wrote:
>
>> flight 21486 xen-unstable real [real]
>> http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/
> [...]
>>   test-armhf-armhf-xl           5 xen-boot                     fail  
never pass
>
> Switching the ARM tests over to Linux 3.12 means we now get much further
> and run into:
>
> (from
http://www.chiark.greenend.org.uk/~xensrcts/logs/21486/test-armhf-armhf-xl/serial-marilith-n5.txt)
>
>> [Wed Nov  6 02:34:54 2013][    1.136164] sata_highbank gpio_request 0
failed: -517
>> [Wed Nov  6 02:34:54 2013][    1.140696] highbank-ahci ffe08000.sata:
AHCI 0001.0300 32 slots 5 ports 3 Gbps 0x1f impl platform mode
>> [Wed Nov  6 02:34:54 2013][    1.149152] highbank-ahci ffe08000.sata:
flags: 64bit ncq sntf stag pm led clo only pmp pio slum part ccc apst
>> [Wed Nov  6 02:34:54 2013][    1.159435] scsi0 : sata_highbank
>> [Wed Nov  6 02:34:54 2013][    1.162451] scsi1 : sata_highbank
>> [Wed Nov  6 02:34:54 2013][    1.165490] scsi2 : sata_highbank
>> [Wed Nov  6 02:34:54 2013][    1.168516] scsi3 : sata_highbank
>> [Wed Nov  6 02:34:54 2013][    1.171566] scsi4 : sata_highbank
>> [Wed Nov  6 02:34:54 2013][    1.174606] ata1: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x100 irq 115
>> [Wed Nov  6 02:34:54 2013][    1.181779] ata2: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x180 irq 115
>> [Wed Nov  6 02:34:54 2013][    1.189060] ata3: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x200 irq 115
>> [Wed Nov  6 02:34:54 2013][    1.196323] ata4: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x280 irq 115
> [...]
>> [Wed Nov  6 02:34:54 2013][    1.203581] ata5: SATA max UDMA/133 mmio
[mem 0xffe08000-0xffe17fff] port 0x300 irq 115
>> [Wed Nov  6 02:35:04 2013][   10.353721] ata1: softreset failed (1st
FIS failed)
>> [Wed Nov  6 02:35:14 2013][   19.362722] ata1: softreset failed (1st
FIS failed)
>> [Wed Nov  6 02:35:49 2013][   50.871721] ata1: softreset failed (1st
FIS failed)
>> [Wed Nov  6 02:35:49 2013][   50.876019] ata1: limiting SATA link speed
to 1.5 Gbps
>> [Wed Nov  6 02:35:54 2013][   55.389720] ata1: softreset failed (1st
FIS failed)
>> [Wed Nov  6 02:35:54 2013][   55.394018] ata1: reset failed, giving up
>> [Wed Nov  6 02:35:55 2013][   55.704724] ata2: SATA link down (SStatus
0 SControl 300)
>> [Wed Nov  6 02:35:55 2013][   56.019724] ata3: SATA link down (SStatus
0 SControl 300)
>> [Wed Nov  6 02:35:55 2013][   56.334723] ata4: SATA link down (SStatus
0 SControl 300)
>> [Wed Nov  6 02:35:56 2013][   56.649722] ata5: SATA link down (SStatus
0 SControl 300)
> [...]
>> [Wed Nov  6 02:35:56 2013]Begin: Mounting root file system ... Begin:
Running /scripts/local-top ...   Volume group "marilith-n5" not found
> ... fail to find root.
>
Hi,

Sorry I''m a bit late to answer.
> Has anyone seen this before, is it a known 3.12 on Midway issue perhaps?
This is due to an interrupt issue. If you apply my non-yet-reworking 
interrupts patch series, all theses errors will disappear.

Cheers,

-- 
Julien Grall

On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Stefano Stabellini wrote:
>
> Hi Stefano,
>
> I haven''t given it too much thought, but here is what I believe
should
> be done:
>
>> The question for you, as arm-soc maintainers, is: do you think this
>> should work and if we find any issues we should just fix them or report
>> them as bugs?
>
> Modifying the DT to mark anything as "reserved" or absent that
Dom0
> should or can not touch sounds like the correct way to do this. Whether
> this needs to be done by modifying the reg property of the device node
> or through a different method I can''t tell.
>
> If you find bugs in the kernel that prevent this from working, but it
> works fine for everyone else, it''s up to you to provide a bug-fix,
> which would most likely be up to Russell to apply.
>
>> Is this entirely going away with multiplatform kernels so we
shouldn''t
>> worry about it?
>
> Multiplatform kernels are by definition relocatable using
> CONFIG_ARM_PATCH_PHYS_VIRT, within some limitations such as the
> granularity of the mapping. You certainly can''t move the start of
memory
> to an address of smaller than 2MB (hugepage) alignment, but you might
> need something larger than that.
During some debugging on the Arndale and Midway, I found another 
constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
I have noticed that all the kernel physical addresses must be lower than 
the corresponding virtual addresses. So the delta offset compute in 
__fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
If this assertion is not validated, when the kernel will browse the 
memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will 
compute a wrong address and will result to consider all memory bank as 
highmem.

After digging in the code, it seems it''s due to some optimization
during
opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>> Or is this a lost fight and should we find a workaround (see below if
we
>> are curious) to make the start of memory look the same?
>
> I don''t see what hack you are referring to, can you elaborate?
>
> My feeling is that we should maintain the requirement that that it must be
> possible to enable Dom0 support on any virtualisation-capable platform
> without breaking other platforms or causing an unreasonable run-time
> overhead.
>
> BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> non-LPAE ARMv6/v7 multiplatform build?
It can be both.

Cheers,

-- 
Julien Grall

On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux
wrote:
> On Tue, Nov 12, 2013 at 12:25:18PM +0000, Stefano Stabellini wrote:
> > Arnd, Olof,
> 
> This isn''t really an arm-soc thing, it''s a core ARM
thing...
> 
> > we have been having this discussion on xen-devel regarding whether Xen
> > should be allowed to modify the start address of the physical memory
> > region in device tree before passing it to dom0 or not.
> > 
> > The reason why this question is coming up now, is that we realized
that
> > we are going to have to live with the 1:1 pseudo-physical to physical
> > mapping for dom0 for a while. This limits the ability of the
hypervisor
> > of allocating dom0 memory wherever it wants. Xen can allocate dom0
> > memory from the low end but maybe not exactly from the start.
> > 
> > As a result we would adjust the start of physical memory in device
tree
> > to match the start of the memory region allocated for dom0. For
example
> > on the Arndale it could be 0x80800000 instead of 0x80000000.
> > 
> > Unfortunately not all the platforms can cope with this very well. In
> > particular the Arndale seems to have issues.
> 
> That should be no problem provided that:
> 
> (a) you load the kernel somewhere between 0x80800000 and 0x80ffffff -
> the decompressor will decide that the start of memory is 0x80800000, and
> place the kernel at 0x80808000.
> (b) _at the moment_ you modify DT to specify that memory starts at
> 0x80800000 and not 0x80000000.
NB we also modify the size.
> (b) is going to change soon: the shmobile and zynq platforms already have
> a problem with their memory setup which needs a patch in this area, and
> the patch will have the side effect of automatically removing (in your
> case) 0x80000000 to 0x80800000.  See the patch below.
I think this is OK. Under Xen we might only give dom0 e.g. 128MB of RAM,
which would be at e.g. 0x808000000-0x88800000, so it sounds like the fix
in question is actually what we would want.
> If there''s any other issues with multiplatform, then yes, we want
to hear
> about them.
I think some of the issues we''ve been seeing were with non-MP kernels.
Specifically they were on Arndale, which appeared to be unhappy with RAM
being much higher than the normal base address. I believe Arndale/Exynos
is currently not (fully?) MP? Or maybe wasn''t when this came up?

Ian.
>  arch/arm/kernel/setup.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index f52150d2ec00..1957d54198ad 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size)
>  	}
>  #endif
>  
> +	if (aligned_start < PHYS_OFFSET) {
> +		if (aligned_start + size < PHYS_OFFSET) {
> +			pr_info("Ignoring memory below PHYS_OFFSET:
0x%08llx-0x%08llx\n",
> +				aligned_start, aligned_start + size);
> +			return -EINVAL;
> +		}
> +
> +		size -= PHYS_OFFSET - aligned_start;
> +		aligned_start = PHYS_OFFSET;
> +	}
> +
>  	bank->start = aligned_start;
>  	bank->size = size & ~(phys_addr_t)(PAGE_SIZE - 1);
>

On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > non-LPAE ARMv6/v7 multiplatform build?
> 
> It can be both.
NB: v7 only, we don''t do v6 at all. But yes either LPAE or regular is
fine with us.

Ian.

On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall
wrote:
> During some debugging on the Arndale and Midway, I found another  
> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> I have noticed that all the kernel physical addresses must be lower than  
> the corresponding virtual addresses. So the delta offset compute in  
> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
> If this assertion is not validated, when the kernel will browse the  
> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will  
> compute a wrong address and will result to consider all memory bank as  
> highmem.
>
> After digging in the code, it seems it''s due to some optimization
during
> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
Are you talking about the code in v3.12 or the code in -next ?

On Tue, Nov 12, 2013 at 02:38:55PM +0000, Ian Campbell
wrote:
> On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux wrote:
> > If there''s any other issues with multiplatform, then yes, we
want to hear
> > about them.
> 
> I think some of the issues we''ve been seeing were with non-MP
kernels.
> Specifically they were on Arndale, which appeared to be unhappy with RAM
> being much higher than the normal base address. I believe Arndale/Exynos
> is currently not (fully?) MP? Or maybe wasn''t when this came up?
I''m not aware of there ever being any SMP/UP constraints on memory in
the
non-platform specific code.

It''s possible that some of the platform specific SMP bringup code
relies
on physical memory at certain addresses (eg, due to restrictions in their
boot protocol for starting secondary CPUs) but running a non-MP kernel
wouldn''t include that code so shouldn''t be affected by this.

On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
>> During some debugging on the Arndale and Midway, I found another
>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>> I have noticed that all the kernel physical addresses must be lower
than
>> the corresponding virtual addresses. So the delta offset compute in
>> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
>> If this assertion is not validated, when the kernel will browse the
>> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
>> compute a wrong address and will result to consider all memory bank as
>> highmem.
>>
>> After digging in the code, it seems it''s due to some
optimization during
>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>
> Are you talking about the code in v3.12 or the code in -next ?
I was talking about 3.12. I have just checked -next and my issue seems 
to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
I should have checked earlier, thanks.

-- 
Julien Grall

On Tue, 2013-11-12 at 14:44 +0000, Russell King - ARM Linux
wrote:
> On Tue, Nov 12, 2013 at 02:38:55PM +0000, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux wrote:
> > > If there''s any other issues with multiplatform, then
yes, we want to hear
> > > about them.
> > 
> > I think some of the issues we''ve been seeing were with non-MP
kernels.
> > Specifically they were on Arndale, which appeared to be unhappy with
RAM
> > being much higher than the normal base address. I believe
Arndale/Exynos
> > is currently not (fully?) MP? Or maybe wasn''t when this came
up?
> 
> I''m not aware of there ever being any SMP/UP constraints on memory
in the
> non-platform specific code.
Sorry MP == multiplatform here. That was a stupid/confusing abbreviation
for me to choose...

Ian.

On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
> 
> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> > On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
> >> During some debugging on the Arndale and Midway, I found another
> >> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> >> I have noticed that all the kernel physical addresses must be
lower than
> >> the corresponding virtual addresses. So the delta offset compute
in
> >> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
> >> If this assertion is not validated, when the kernel will browse
the
> >> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...)
will
> >> compute a wrong address and will result to consider all memory
bank as
> >> highmem.
> >>
> >> After digging in the code, it seems it''s due to some
optimization during
> >> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
> >
> > Are you talking about the code in v3.12 or the code in -next ?
> 
> I was talking about 3.12. I have just checked -next and my issue seems 
> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
> I should have checked earlier, thanks.
Should we revert your Xen side fix^Wworkaround then:

commit 6c21cb36e263de2db8716b477157a5b6cd531e1e
Author: Julien Grall <julien.grall@linaro.org>
Date:   Tue Oct 22 11:51:48 2013 +0100

    xen/arm: Allocate memory for dom0 from the bottom with the 1:1 Workaround
    
    On Linux, the option CONFIG_ARM_PATCH_PHYS_VIRT (by default enabled) allow
    the Kernel to be loaded anywhere (or nearly) by patching the translation
    pv<->virt at boot time.
    
    The current solution in Linux assuming that the delta physical address -
    virtual address is always negative. A positive delta will destroy all the
    optimisation to modify only a part of the translation instruction (add/sub
    
    By default, Xen is allocating memory from the top of memory and then
    goes down. To avoid booting issue with Linux, we must allocate memory
    from the bottom (ie starting from 0).
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>

The plus side of reverting it is that we can help find any future issues
with Linux. The down side of reverting it is that we can help find any
future issues with Linux...

Ian.

On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > Or is this a lost fight and should we find a workaround (see below if
we
> > are curious) to make the start of memory look the same?
> 
> I don''t see what hack you are referring to, can you elaborate?
The hack would be mapping dom0 memory at the same start address as it
would be on the native platform but allocating its memory contiguously
at an offset.

For example having dom0 psedo-physical memory start at 0x80000000 (as it
would expect) and end at 0x88000000 (because let''s sat that we only
give
128MB of memory to dom0). The actual physical memory would be allocated
contiguously from 0xA0000000. The swiotlb-xen code in Linux would be
made aware of the offset 0xA0000000-0x80000000 via device tree and
would calculate the right physical address to use for dma operations.
Today swiotlb-xen assumes that pseudo-phisical addresses correspond to
physical addresses for dom0.

It feels to me like an hack to work around Linux limitations.

On Tue, Nov 12, 2013 at 03:00:40PM +0000, Stefano Stabellini
wrote:
> On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > Or is this a lost fight and should we find a workaround (see
below if we
> > > are curious) to make the start of memory look the same?
> > 
> > I don''t see what hack you are referring to, can you
elaborate?
> 
> The hack would be mapping dom0 memory at the same start address as it
> would be on the native platform but allocating its memory contiguously
> at an offset.
> 
> For example having dom0 psedo-physical memory start at 0x80000000 (as it
> would expect) and end at 0x88000000 (because let''s sat that we
only give
> 128MB of memory to dom0). The actual physical memory would be allocated
> contiguously from 0xA0000000. The swiotlb-xen code in Linux would be
> made aware of the offset 0xA0000000-0x80000000 via device tree and
> would calculate the right physical address to use for dma operations.
> Today swiotlb-xen assumes that pseudo-phisical addresses correspond to
> physical addresses for dom0.
First, let''s get something right here.  Conceptually, DMA addresses
have
_never_ been the same as physical addresses in the kernel (with the
exception of DMA masks being interpreted strangely.)

Physical addresses have always been what''s required to be programmed
into
things like the MMU to gain access to RAM.  We have virt_to_phys() etc
for translations of this nature for lowmem, and kmap() etc for highmem.

DMA addresses have always been the value which you program into the DMA
controller for it to be able to access RAM.  These use the DMA API to
obtain the DMA address.  Behind the DMA API, we have dma_to_virt() and
virt_to_dma() etc which do the appropriate translation for this.

Of course, for virtualisation, replacing the DMA ops is probably the
better solution to deal with this, where you can hook into the mapping/
unmapping/coherent functions and return the correct DMA addresses.

Remember though - anything which assumes virtual_address
phys_to_virt(dma_addr_t) is basically violating the conceptual model
here, and should be fixed.

On 11/12/2013 02:57 PM, Ian Campbell wrote:
> On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
>>
>> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
>>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
>>>> During some debugging on the Arndale and Midway, I found
another
>>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>>>> I have noticed that all the kernel physical addresses must be
lower than
>>>> the corresponding virtual addresses. So the delta offset
compute in
>>>> __fixup_pv_table (arch/arm/kernel/head.S) must always be
negative.
>>>> If this assertion is not validated, when the kernel will browse
the
>>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c),
__phys(...) will
>>>> compute a wrong address and will result to consider all memory
bank as
>>>> highmem.
>>>>
>>>> After digging in the code, it seems it''s due to some
optimization during
>>>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>>>
>>> Are you talking about the code in v3.12 or the code in -next ?
>>
>> I was talking about 3.12. I have just checked -next and my issue seems
>> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
>> I should have checked earlier, thanks.
>
> Should we revert your Xen side fix^Wworkaround then:
For now it''s only in -next. I would wait until this commit is at least 
in Linux master, otherwise we will likely break vanilla/distro kernel 
for Xen 4.4.

-- 
Julien Grall

On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 12, 2013 at 12:25:18PM +0000, Stefano Stabellini wrote:
>> Arnd, Olof,
> 
> This isn''t really an arm-soc thing, it''s a core ARM
thing...
> 
>> we have been having this discussion on xen-devel regarding whether Xen
>> should be allowed to modify the start address of the physical memory
>> region in device tree before passing it to dom0 or not.
>>
>> The reason why this question is coming up now, is that we realized that
>> we are going to have to live with the 1:1 pseudo-physical to physical
>> mapping for dom0 for a while. This limits the ability of the hypervisor
>> of allocating dom0 memory wherever it wants. Xen can allocate dom0
>> memory from the low end but maybe not exactly from the start.
>>
>> As a result we would adjust the start of physical memory in device tree
>> to match the start of the memory region allocated for dom0. For example
>> on the Arndale it could be 0x80800000 instead of 0x80000000.
>>
>> Unfortunately not all the platforms can cope with this very well. In
>> particular the Arndale seems to have issues.
> 
> That should be no problem provided that:
> 
> (a) you load the kernel somewhere between 0x80800000 and 0x80ffffff -
> the decompressor will decide that the start of memory is 0x80800000, and
> place the kernel at 0x80808000.
> (b) _at the moment_ you modify DT to specify that memory starts at
> 0x80800000 and not 0x80000000.
> 
> (b) is going to change soon: the shmobile and zynq platforms already have
> a problem with their memory setup which needs a patch in this area, and
> the patch will have the side effect of automatically removing (in your
> case) 0x80000000 to 0x80800000.  See the patch below.
I wouldn''t say problem here - just use case where we want/need to
ignore
the part of memory. The patch below works nicely for us.
> 
> If there''s any other issues with multiplatform, then yes, we want
to hear
> about them.
> 
>  arch/arm/kernel/setup.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index f52150d2ec00..1957d54198ad 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size)
>  	}
>  #endif
>  
> +	if (aligned_start < PHYS_OFFSET) {
> +		if (aligned_start + size < PHYS_OFFSET) {
just a note I sent to Russell. Here should be "<=" instead of just
"<"
> +			pr_info("Ignoring memory below PHYS_OFFSET:
0x%08llx-0x%08llx\n",
> +				aligned_start, aligned_start + size);
> +			return -EINVAL;
> +		}
> +
> +		size -= PHYS_OFFSET - aligned_start;
> +		aligned_start = PHYS_OFFSET;
We should printk a message here to be aware that alignment was done.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Tue, 2013-11-12 at 15:24 +0000, Julien Grall wrote:
> 
> On 11/12/2013 02:57 PM, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
> >>
> >> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> >>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
> >>>> During some debugging on the Arndale and Midway, I found
another
> >>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> >>>> I have noticed that all the kernel physical addresses must
be lower than
> >>>> the corresponding virtual addresses. So the delta offset
compute in
> >>>> __fixup_pv_table (arch/arm/kernel/head.S) must always be
negative.
> >>>> If this assertion is not validated, when the kernel will
browse the
> >>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c),
__phys(...) will
> >>>> compute a wrong address and will result to consider all
memory bank as
> >>>> highmem.
> >>>>
> >>>> After digging in the code, it seems it''s due to
some optimization during
> >>>> opcode fixup in __fixup_a_pvtable. Is it a wanted
constraint?
> >>>
> >>> Are you talking about the code in v3.12 or the code in -next ?
> >>
> >> I was talking about 3.12. I have just checked -next and my issue
seems
> >> to be fixed by the commit
f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
> >> I should have checked earlier, thanks.
> >
> > Should we revert your Xen side fix^Wworkaround then:
> 
> For now it''s only in -next. I would wait until this commit is at
least
> in Linux master, otherwise we will likely break vanilla/distro kernel 
> for Xen 4.4.
OK. Can you keep an eye on it and let me know if/when the time comes to
revert?

Is the fix a candidate for stable? Seems like a bit of a big exciting
fix for what was an quite recently an obscure corner case...

Ian.

On Tue, Nov 12, 2013 at 04:24:46PM +0100, Michal Simek
wrote:
> On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote:
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index f52150d2ec00..1957d54198ad 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size)
> >  	}
> >  #endif
> >  
> > +	if (aligned_start < PHYS_OFFSET) {
> > +		if (aligned_start + size < PHYS_OFFSET) {
> 
> just a note I sent to Russell. Here should be "<=" instead of
just "<"
> 
> > +			pr_info("Ignoring memory below PHYS_OFFSET:
0x%08llx-0x%08llx\n",
> > +				aligned_start, aligned_start + size);
> > +			return -EINVAL;
> > +		}
> > +
> > +		size -= PHYS_OFFSET - aligned_start;
> > +		aligned_start = PHYS_OFFSET;
> 
> We should printk a message here to be aware that alignment was done.
Yep, you sent me an updated patch, I haven''t added anything to my
kernel tree for about a week and a bit now as I want to get what''s
already there into Linus'' tree.  Once that stuff has gone,
I''ll see
about adding some of these fixes we need into the queue.

On 11/12/2013 04:39 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 12, 2013 at 04:24:46PM +0100, Michal Simek wrote:
>> On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote:
>>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>>> index f52150d2ec00..1957d54198ad 100644
>>> --- a/arch/arm/kernel/setup.c
>>> +++ b/arch/arm/kernel/setup.c
>>> @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size)
>>>  	}
>>>  #endif
>>>  
>>> +	if (aligned_start < PHYS_OFFSET) {
>>> +		if (aligned_start + size < PHYS_OFFSET) {
>>
>> just a note I sent to Russell. Here should be "<=" instead
of just "<"
>>
>>> +			pr_info("Ignoring memory below PHYS_OFFSET:
0x%08llx-0x%08llx\n",
>>> +				aligned_start, aligned_start + size);
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		size -= PHYS_OFFSET - aligned_start;
>>> +		aligned_start = PHYS_OFFSET;
>>
>> We should printk a message here to be aware that alignment was done.
> 
> Yep, you sent me an updated patch, I haven''t added anything to my
> kernel tree for about a week and a bit now as I want to get what''s
> already there into Linus'' tree.  Once that stuff has gone,
I''ll see
> about adding some of these fixes we need into the queue.
No problem. Take you time.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




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

On Tuesday 12 November 2013, Ian Campbell wrote:
> On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a
regular
> > > non-LPAE ARMv6/v7 multiplatform build?
> > 
> > It can be both.
> 
> NB: v7 only, we don''t do v6 at all. But yes either LPAE or regular
is
> fine with us.
Why not combined v6/v7 kernels for non-LPAE? I can''t see a technical
reason
preventing you from running a Dom0 or DomU kernel that can also run on
some ARMv6 platform as long as both platforms and CPUs are enabled in
Kconfig.

	Arnd

On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a
regular
> > > > non-LPAE ARMv6/v7 multiplatform build?
> > > 
> > > It can be both.
> > 
> > NB: v7 only, we don''t do v6 at all. But yes either LPAE or
regular is
> > fine with us.
> 
> Why not combined v6/v7 kernels for non-LPAE? I can''t see a
technical reason
> preventing you from running a Dom0 or DomU kernel that can also run on
> some ARMv6 platform as long as both platforms and CPUs are enabled in
> Kconfig.
Unfortunately today we can''t support ARMv6.
From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
   
    * ARMv6 does not support cmpxchg on 16-bit words that are used in the
      Xen grant table code, so we must ensure that Xen support is only
      built on ARMv7-only kernels not combined ARMv6/v7 kernels.

On Tuesday 12 November 2013, Stefano Stabellini wrote:
> On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > BTW, does Dom0 require an LPAE-enabled kernel or can it
be a regular
> > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > 
> > > > It can be both.
> > > 
> > > NB: v7 only, we don''t do v6 at all. But yes either LPAE
or regular is
> > > fine with us.
> > 
> > Why not combined v6/v7 kernels for non-LPAE? I can''t see a
technical reason
> > preventing you from running a Dom0 or DomU kernel that can also run on
> > some ARMv6 platform as long as both platforms and CPUs are enabled in
> > Kconfig.
> 
> Unfortunately today we can''t support ARMv6.
> From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
>    
>     * ARMv6 does not support cmpxchg on 16-bit words that are used in the
>       Xen grant table code, so we must ensure that Xen support is only
>       built on ARMv7-only kernels not combined ARMv6/v7 kernels.
Ah, I must have made a mistake there. It''s not strictly a bug, but I
think
it would be better to undo the dependency I added in that patch and instead
change the Makefile to build the grant table code with -march=armv7-a:
This is safe because we know that this code will only /run/ on v7 even
in a combined v6/v7 kernel, but it lets us get better build coverage because
then we will enable Xen support in an allmodconfig or allyesconfig kernel
that today enables both v6 and v7.

	Arnd

On Tue, 2013-11-12 at 21:08 +0100, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Stefano Stabellini wrote:
> > On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > > BTW, does Dom0 require an LPAE-enabled kernel or
can it be a regular
> > > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > > 
> > > > > It can be both.
> > > > 
> > > > NB: v7 only, we don''t do v6 at all. But yes either
LPAE or regular is
> > > > fine with us.
> > > 
> > > Why not combined v6/v7 kernels for non-LPAE? I can''t see
a technical reason
> > > preventing you from running a Dom0 or DomU kernel that can also
run on
> > > some ARMv6 platform as long as both platforms and CPUs are
enabled in
> > > Kconfig.
> > 
> > Unfortunately today we can''t support ARMv6.
> > From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
> >    
> >     * ARMv6 does not support cmpxchg on 16-bit words that are used in
the
> >       Xen grant table code, so we must ensure that Xen support is only
> >       built on ARMv7-only kernels not combined ARMv6/v7 kernels.
> 
> Ah, I must have made a mistake there. It''s not strictly a bug, but
I think
> it would be better to undo the dependency I added in that patch and instead
> change the Makefile to build the grant table code with -march=armv7-a:
> This is safe because we know that this code will only /run/ on v7 even
> in a combined v6/v7 kernel, but it lets us get better build coverage
because
> then we will enable Xen support in an allmodconfig or allyesconfig kernel
> that today enables both v6 and v7.
> 
This seems reasonable to me if it can be made to work. e.g. the uses of
such constructs would need to be in .c files not static inlines in .h
for it not to get ugly fast. Hopefully that is the case.

Another thing to watch out for is the atomics in xchg_xen_ulong which is
used by drivers/xen/events.c and uses atomic64_xchg expecting to get
exclusive load/store instructions. it looks to me like atomic64_xchg is
the same for v6 and v7 so that is ok.

The last thing to watch out for is sync_test_bit/_test_and_set etc.
Again those look the same to me on v6 and v7.

Ian.

On 11/12/2013 03:32 PM, Ian Campbell wrote:
> On Tue, 2013-11-12 at 15:24 +0000, Julien Grall wrote:
>>
>> On 11/12/2013 02:57 PM, Ian Campbell wrote:
>>> On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
>>>>
>>>> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
>>>>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall
wrote:
>>>>>> During some debugging on the Arndale and Midway, I
found another
>>>>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>>>>>> I have noticed that all the kernel physical addresses
must be lower than
>>>>>> the corresponding virtual addresses. So the delta
offset compute in
>>>>>> __fixup_pv_table (arch/arm/kernel/head.S) must always
be negative.
>>>>>> If this assertion is not validated, when the kernel
will browse the
>>>>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c),
__phys(...) will
>>>>>> compute a wrong address and will result to consider all
memory bank as
>>>>>> highmem.
>>>>>>
>>>>>> After digging in the code, it seems it''s due
to some optimization during
>>>>>> opcode fixup in __fixup_a_pvtable. Is it a wanted
constraint?
>>>>>
>>>>> Are you talking about the code in v3.12 or the code in
-next ?
>>>>
>>>> I was talking about 3.12. I have just checked -next and my
issue seems
>>>> to be fixed by the commit
f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
>>>> I should have checked earlier, thanks.
>>>
>>> Should we revert your Xen side fix^Wworkaround then:
>>
>> For now it''s only in -next. I would wait until this commit is
at least
>> in Linux master, otherwise we will likely break vanilla/distro kernel
>> for Xen 4.4.
>
> OK. Can you keep an eye on it and let me know if/when the time comes to
> revert?
No problem.
>
> Is the fix a candidate for stable? Seems like a bit of a big exciting
> fix for what was an quite recently an obscure corner case...
I think yes, as soon as Linux contains this fix we will be able to 
revert the commit in Xen.

-- 
Julien Grall

On Wed, 13 Nov 2013, Ian Campbell wrote:
> On Tue, 2013-11-12 at 21:08 +0100, Arnd Bergmann wrote:
> > On Tuesday 12 November 2013, Stefano Stabellini wrote:
> > > On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > > > BTW, does Dom0 require an LPAE-enabled kernel
or can it be a regular
> > > > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > > > 
> > > > > > It can be both.
> > > > > 
> > > > > NB: v7 only, we don''t do v6 at all. But yes
either LPAE or regular is
> > > > > fine with us.
> > > > 
> > > > Why not combined v6/v7 kernels for non-LPAE? I
can''t see a technical reason
> > > > preventing you from running a Dom0 or DomU kernel that can
also run on
> > > > some ARMv6 platform as long as both platforms and CPUs are
enabled in
> > > > Kconfig.
> > > 
> > > Unfortunately today we can''t support ARMv6.
> > > From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
> > >    
> > >     * ARMv6 does not support cmpxchg on 16-bit words that are
used in the
> > >       Xen grant table code, so we must ensure that Xen support is
only
> > >       built on ARMv7-only kernels not combined ARMv6/v7 kernels.
> > 
> > Ah, I must have made a mistake there. It''s not strictly a
bug, but I think
> > it would be better to undo the dependency I added in that patch and
instead
> > change the Makefile to build the grant table code with -march=armv7-a:
> > This is safe because we know that this code will only /run/ on v7 even
> > in a combined v6/v7 kernel, but it lets us get better build coverage
because
> > then we will enable Xen support in an allmodconfig or allyesconfig
kernel
> > that today enables both v6 and v7.
> > 
> 
> This seems reasonable to me if it can be made to work. e.g. the uses of
> such constructs would need to be in .c files not static inlines in .h
> for it not to get ugly fast. Hopefully that is the case.
> 
> Another thing to watch out for is the atomics in xchg_xen_ulong which is
> used by drivers/xen/events.c and uses atomic64_xchg expecting to get
> exclusive load/store instructions. it looks to me like atomic64_xchg is
> the same for v6 and v7 so that is ok.
> 
> The last thing to watch out for is sync_test_bit/_test_and_set etc.
> Again those look the same to me on v6 and v7.
It is more complicated than I expected as XEN depends on
!GENERIC_ATOMIC64 because we require proper atomic instructions to
read/write memory shared with the hypervisor in events.c (see
85323a991d40681023822e86ca95f38a75262026).
Unfortunately config ARM selects GENERIC_ATOMIC64 if CPU_V6.

In addition to modifying arch/arm/Kconfig and drivers/xen/Makefile I had
to remove the XEN dependency on !GENERIC_ATOMIC64 by reimplementing
atomic64_xchg.

Finally the cmpxchg code used by grant-table.c is in a static inline
protected by #ifndef CONFIG_CPU_V6 (see arch/arm/include/asm/cmpxchg.h).
Passing -march=armv7-a from the Makefile is not enough.


---


diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01f7013..3a888e1 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1885,8 +1885,7 @@ config XEN_DOM0
 config XEN
 	bool "Xen guest support on ARM (EXPERIMENTAL)"
 	depends on ARM && AEABI && OF
-	depends on CPU_V7 && !CPU_V6
-	depends on !GENERIC_ATOMIC64
+	depends on CPU_V7
 	select ARM_PSCI
 	select SWIOTLB_XEN
 	help
diff --git a/arch/arm/include/asm/xen/events.h
b/arch/arm/include/asm/xen/events.h
index 8b1f37b..2032ee6 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -16,7 +16,37 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
 	return raw_irqs_disabled_flags(regs->ARM_cpsr);
 }
 
-#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
+#ifdef CONFIG_GENERIC_ATOMIC64
+/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
+ * atomic64_xchg function because it is implemented using spin locks.
+ * Here we need proper atomic instructions to read and write memory
+ * shared with the hypervisor.
+ */
+static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
+{
+	u64 result;
+	unsigned long tmp;
+
+	smp_mb();
+
+	__asm__ __volatile__("@ xen_atomic64_xchg\n"
+"1:	ldrexd	%0, %H0, [%3]\n"
+"	strexd	%1, %4, %H4, [%3]\n"
+"	teq	%1, #0\n"
+"	bne	1b"
+	: "=&r" (result), "=&r" (tmp), "+Qo"
(ptr->counter)
+	: "r" (&ptr->counter), "r" (new)
+	: "cc");
+
+	smp_mb();
+
+	return result;
+}
+#else
+#define xen_atomic64_xchg atomic64_xchg
+#endif
+
+#define xchg_xen_ulong(ptr, val) xen_atomic64_xchg(container_of((ptr),	\
 							    atomic64_t,	\
 							    counter), (val))
 
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 62ccf54..d668c3c 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -33,6 +33,14 @@
 
 #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
 
+/* This is required because cmpxchg only support 32-bits operands on
+ * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention will
+ * be limited to 32-bits operands.
+ * However we know for sure that if Linux is running on Xen, the
+ * platform is >= ARMv7, so here we can safely undef CONFIG_CPU_V6.
+ */
+#undef CONFIG_CPU_V6
+
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/mm.h>

On Wednesday 13 November 2013, Stefano Stabellini wrote:
>  
> -#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
> +#ifdef CONFIG_GENERIC_ATOMIC64
> +/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
> + * atomic64_xchg function because it is implemented using spin locks.
> + * Here we need proper atomic instructions to read and write memory
> + * shared with the hypervisor.
> + */
> +static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
> +{
> +	u64 result;
> +	unsigned long tmp;
> +
> +	smp_mb();
> +
> +	__asm__ __volatile__("@ xen_atomic64_xchg\n"
> +"1:	ldrexd	%0, %H0, [%3]\n"
> +"	strexd	%1, %4, %H4, [%3]\n"
> +"	teq	%1, #0\n"
> +"	bne	1b"
> +	: "=&r" (result), "=&r" (tmp),
"+Qo" (ptr->counter)
> +	: "r" (&ptr->counter), "r" (new)
> +	: "cc");
> +
> +	smp_mb();
> +
> +	return result;
> +}
> +#else
> +#define xen_atomic64_xchg atomic64_xchg
> +#endif
I would prefer not duplicating this code. Maybe we can instead change
the ARM code to always provide this function under the name of
armv7_atomic64_xchg() and conditionally defining atomic64_xchg
to use it. Let''s see if Russell has an opinion on this, or perhaps
a better solution.
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 62ccf54..d668c3c 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -33,6 +33,14 @@
>  
>  #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
>  
> +/* This is required because cmpxchg only support 32-bits operands on
> + * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention will
> + * be limited to 32-bits operands.
> + * However we know for sure that if Linux is running on Xen, the
> + * platform is >= ARMv7, so here we can safely undef CONFIG_CPU_V6.
> + */
> +#undef CONFIG_CPU_V6
> +
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <linux/mm.h>
> 
Same here: I''d prefer making this more explicit by changing the cmpxchg
implementation: we could split out the 16-bit cmpxchg case into a separate
function with "armv7" in the name and only call that from the main
cmpxchg() implementations when building an armv7-only kernel, but still
let you call it from Xen code that is known to run only on armv7.

	Arnd

On Wed, 13 Nov 2013, Arnd Bergmann wrote:
> On Wednesday 13 November 2013, Stefano Stabellini wrote:
> >  
> > -#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
> > +#ifdef CONFIG_GENERIC_ATOMIC64
> > +/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
> > + * atomic64_xchg function because it is implemented using spin locks.
> > + * Here we need proper atomic instructions to read and write memory
> > + * shared with the hypervisor.
> > + */
> > +static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
> > +{
> > +	u64 result;
> > +	unsigned long tmp;
> > +
> > +	smp_mb();
> > +
> > +	__asm__ __volatile__("@ xen_atomic64_xchg\n"
> > +"1:	ldrexd	%0, %H0, [%3]\n"
> > +"	strexd	%1, %4, %H4, [%3]\n"
> > +"	teq	%1, #0\n"
> > +"	bne	1b"
> > +	: "=&r" (result), "=&r" (tmp),
"+Qo" (ptr->counter)
> > +	: "r" (&ptr->counter), "r" (new)
> > +	: "cc");
> > +
> > +	smp_mb();
> > +
> > +	return result;
> > +}
> > +#else
> > +#define xen_atomic64_xchg atomic64_xchg
> > +#endif
> 
> I would prefer not duplicating this code. Maybe we can instead change
> the ARM code to always provide this function under the name of
> armv7_atomic64_xchg() and conditionally defining atomic64_xchg
> to use it. Let''s see if Russell has an opinion on this, or perhaps
> a better solution.
> 
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 62ccf54..d668c3c 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -33,6 +33,14 @@
> >  
> >  #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": "
fmt
> >  
> > +/* This is required because cmpxchg only support 32-bits operands on
> > + * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention
will
> > + * be limited to 32-bits operands.
> > + * However we know for sure that if Linux is running on Xen, the
> > + * platform is >= ARMv7, so here we can safely undef
CONFIG_CPU_V6.
> > + */
> > +#undef CONFIG_CPU_V6
> > +
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <linux/mm.h>
> > 
> 
> Same here: I''d prefer making this more explicit by changing the
cmpxchg
> implementation: we could split out the 16-bit cmpxchg case into a separate
> function with "armv7" in the name and only call that from the
main
> cmpxchg() implementations when building an armv7-only kernel, but still
> let you call it from Xen code that is known to run only on armv7.
 
what do you think of:

http://marc.info/?l=linux-arm-kernel&m=138444245625671&w=2

?