On 07.08.2017 22:35, Michael S. Tsirkin wrote:
> On Thu, Aug 03, 2017 at 03:30:19PM +0200, Thomas Huth wrote:
>> Now that we've got a firmware that can do TFTP booting on s390x (i.e.
>> the pc-bios/s390-netboot.img), we can enable the PXE tester for this
>> architecture, too.
>>
>> Signed-off-by: Thomas Huth <th...@redhat.com>
>> ---
>> Since this only adds a test, I guess this could still be included
>> for 2.10 ... otherwise, I think it's also OK to simply postpone this
>> to 2.11 instead
>
> I think 2.11 is preferable.
>
>> tests/Makefile.include | 1 +
>> tests/boot-sector.c | 25 ++++++++++++++++++++++---
>> tests/pxe-test.c | 7 +++++++
>> 3 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 59e536b..f9af5e3 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -337,6 +337,7 @@ check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
>>
>> check-qtest-s390x-y = tests/boot-serial-test$(EXESUF)
>> +check-qtest-s390x-y += tests/pxe-test$(EXESUF)
>>
>> check-qtest-generic-y += tests/qom-test$(EXESUF)
>> check-qtest-generic-y += tests/test-hmp$(EXESUF)
>> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
>> index e3880f4..87a8fb5 100644
>> --- a/tests/boot-sector.c
>> +++ b/tests/boot-sector.c
>> @@ -21,6 +21,7 @@
>> #define SIGNATURE 0xdead
>> #define SIGNATURE_OFFSET 0x10
>> #define BOOT_SECTOR_ADDRESS 0x7c00
>> +#define SIGNATURE_ADDR (BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET)
>>
>> /* Boot sector code: write SIGNATURE into memory,
>> * then halt.
>> @@ -73,6 +74,7 @@ int boot_sector_init(char *fname)
>> {
>> int fd, ret;
>> size_t len = sizeof boot_sector;
>> + const char *arch = qtest_get_arch();
>>
>> fd = mkstemp(fname);
>> if (fd < 0) {
>> @@ -81,10 +83,27 @@ int boot_sector_init(char *fname)
>> }
>>
>> /* For Open Firmware based system, we can use a Forth script instead */
>> - if (strcmp(qtest_get_arch(), "ppc64") == 0) {
>> + if (g_str_equal(arch, "ppc64")) {
>> len = sprintf((char *)boot_sector, "\\ Bootscript\n%x %x c! %x %x
>> c!\n",
>> - LOW(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET,
>> - HIGH(SIGNATURE), BOOT_SECTOR_ADDRESS + SIGNATURE_OFFSET +
>> 1);
>> + LOW(SIGNATURE), SIGNATURE_ADDR,
>> + HIGH(SIGNATURE), SIGNATURE_ADDR + 1);
>> + } else if (g_str_equal(arch, "s390x")) {
>> + /* For s390x, fake a kernel signature */
>> + const uint8_t psw[] = {
>> + 0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
>> + };
>> + const uint8_t code[] = {
>> + 0xa7, 0xf4, 0x00, 0x0a, /* j 0x10010 */
>> + 0x00, 0x00, 0x00, 0x00,
>> + 'S', '3', '9', '0',
>> + 'E', 'P', 0x00, 0x01,
>> + 0xa7, 0x38, HIGH(SIGNATURE_ADDR), LOW(SIGNATURE_ADDR), /* lhi
>> r3 */
>> + 0xa7, 0x48, LOW(SIGNATURE), HIGH(SIGNATURE), /* lhi
>> r4,0xadde */
>> + 0x40, 0x40, 0x30, 0x00, /* sth r4,0(r3) */
>> + 0xa7, 0xf4, 0xff, 0xfa /* j 0x10010 */
>> + };
>> + memcpy(boot_sector, psw, 8);
>> + memcpy(&boot_sector[0x10000], code, sizeof(code));
>
>
> Could we avoid overwriting boot sector?
> We really should have
> x86_boot_sector
> ppc64_boot_sector
> s390_boot_sector
>
> And write out the correct thing.
Ok, I don't mind either way ... I can try to come up with a patch.
> Also:
> * Q35 machine requires a minimum 0x7e000 bytes disk.
> * (bug or feature?)
>
> probably does not apply to s390x, does it?
We can also just load 0x10000 + sizeof(code) bytes here. I'll fix it.
Cornelia, please unqueue the patch, I'll send a v2...
Thomas
