| Message ID | 1392283031-40129-3-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 13 Feb 2014 10:17:10 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > We have to set the cssid to 0, otherwise the stsch code will > return an operand exception without the m bit. In the same way > we should set m=0. > > This case was triggered in some cases during reboot, if for some > reason the location of blk_schid.cssid contains 1 and m was 0. > Turns out that the qemu elf loader does not zero out the bss section. > On first boot this is ok, but on reboots this might fail. > > The symptom was an dump of the old kernel with several areas > overwritten. The bootloader does not register a program check > handler, so bios exception jumped back into the old kernel. > > Lets just use a local struct with a designed initializer. That > will guarantee that all other subelements are initialized to 0. > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > pc-bios/s390-ccw/main.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
On 13 February 2014 09:17, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > static void virtio_setup(uint64_t dev_info) > { > + struct subchannel_id blk_schid = { .one = 1}; Missing space before the "}" I think. thanks -- PMM
On 13/02/14 10:55, Peter Maydell wrote: > On 13 February 2014 09:17, Christian Borntraeger <borntraeger@de.ibm.com> wrote: >> static void virtio_setup(uint64_t dev_info) >> { >> + struct subchannel_id blk_schid = { .one = 1}; > > Missing space before the "}" I think. checkpatch accepts both ways: a) struct subchannel_id blk_schid = { .one = 1}; b) struct subchannel_id blk_schid = { .one = 1 }; so, change it or keep it? Christian
On 13 February 2014 10:05, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 13/02/14 10:55, Peter Maydell wrote: >> On 13 February 2014 09:17, Christian Borntraeger <borntraeger@de.ibm.com> wrote: >>> static void virtio_setup(uint64_t dev_info) >>> { >>> + struct subchannel_id blk_schid = { .one = 1}; >> >> Missing space before the "}" I think. > > checkpatch accepts both ways: > a) struct subchannel_id blk_schid = { .one = 1}; > b) struct subchannel_id blk_schid = { .one = 1 }; > > so, change it or keep it? checkpatch isn't infallible. I think having the space looks better. In any case you should be consistent about whether you use a space with both the opening and the closing brace -- at the moment you've got a space at one end and not the other. thanks -- PMM
On 13/02/14 12:04, Peter Maydell wrote: > On 13 February 2014 10:05, Christian Borntraeger <borntraeger@de.ibm.com> wrote: >> On 13/02/14 10:55, Peter Maydell wrote: >>> On 13 February 2014 09:17, Christian Borntraeger <borntraeger@de.ibm.com> wrote: >>>> static void virtio_setup(uint64_t dev_info) >>>> { >>>> + struct subchannel_id blk_schid = { .one = 1}; >>> >>> Missing space before the "}" I think. >> >> checkpatch accepts both ways: >> a) struct subchannel_id blk_schid = { .one = 1}; >> b) struct subchannel_id blk_schid = { .one = 1 }; >> >> so, change it or keep it? > > checkpatch isn't infallible. I think having the space > looks better. In any case you should be consistent > about whether you use a space with both the opening > and the closing brace -- at the moment you've got a > space at one end and not the other. > > thanks Ok changed. Will wait for some more feedback and send an updated pull requests. Christian
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index c5d5332..d7fc727 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -10,7 +10,6 @@ #include "s390-ccw.h" -struct subchannel_id blk_schid; char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); uint64_t boot_value; @@ -23,13 +22,13 @@ void virtio_panic(const char *string) static void virtio_setup(uint64_t dev_info) { + struct subchannel_id blk_schid = { .one = 1}; struct schib schib; int i; int r; bool found = false; bool check_devno = false; uint16_t dev_no = -1; - blk_schid.one = 1; if (dev_info != -1) { check_devno = true;
We have to set the cssid to 0, otherwise the stsch code will return an operand exception without the m bit. In the same way we should set m=0. This case was triggered in some cases during reboot, if for some reason the location of blk_schid.cssid contains 1 and m was 0. Turns out that the qemu elf loader does not zero out the bss section. On first boot this is ok, but on reboots this might fail. The symptom was an dump of the old kernel with several areas overwritten. The bootloader does not register a program check handler, so bios exception jumped back into the old kernel. Lets just use a local struct with a designed initializer. That will guarantee that all other subelements are initialized to 0. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- pc-bios/s390-ccw/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)