Message ID | 20240226000259.2752893-1-sergey.kambalin@auriga.com (mailing list archive) |
---|---|
Headers | show |
Series | Raspberry Pi 4B machine | expand |
On Mon, 26 Feb 2024 at 00:04, Sergey Kambalin <serg.oker@gmail.com> wrote: > > Introducing Raspberry Pi 4B model. > It contains new BCM2838 SoC, PCIE subsystem, > RNG200, Thermal sensor and Genet network controller. > > It can work with recent linux kernels 6.x.x. > Two avocado tests was added to check that. > > Unit tests has been made as read/write operations > via mailbox properties. > > Genet integration test is under development. > > Every single commit > 1) builds without errors > 2) passes regression tests > 3) passes style check* > *the only exception is bcm2838-mbox-property-test.c file > containing heavy macros usage which cause a lot of > false-positives of checkpatch.pl. > > I did my best to keep the commits less than 200 changes, > but had to make some of them a bit more in order to > keep their integrity. > > > Sergey Kambalin (41): > Split out common part of BCM283X classes > Split out common part of peripherals > Split out raspi machine common part > Introduce BCM2838 SoC > Add GIC-400 to BCM2838 SoC > Add BCM2838 GPIO stub > Implement BCM2838 GPIO functionality I've just noticed that the commit messages in this series are missing the conventional prefix that indicates what part of the codebase they apply to (hw/arm, hw/gpio, etc). I propose to add those in on my end for the patches I'm taking into target-arm.next. I think the one question I have left is the name of the board: currently it's "raspi4b-2g", but should we name it just "raspi4b"? None of the names we use for the other raspi boards we model have a suffix like the "-2g" here. Philippe, do you have an opinion here ? -- PMM
On Mon, 26 Feb 2024 at 18:09, Peter Maydell <peter.maydell@linaro.org> wrote: > I think the one question I have left is the name of the > board: currently it's "raspi4b-2g", but should we name > it just "raspi4b"? None of the names we use for the other > raspi boards we model have a suffix like the "-2g" here. > Philippe, do you have an opinion here ? I had a look at what the different raspi4 models are, and it looks like they're all the same except for the amount of RAM. For QEMU, if we wanted to allow users to pick the RAM size we'd do it via the -m option, not by using different model names. So I think on that basis and to match the existing raspi machine names, we should call this simply "raspi4b". So I'll make that name change in the patches I have in target-arm.next. thanks -- PMM
On Mon, 26 Feb 2024 at 00:04, Sergey Kambalin <serg.oker@gmail.com> wrote: > > Introducing Raspberry Pi 4B model. > It contains new BCM2838 SoC, PCIE subsystem, > RNG200, Thermal sensor and Genet network controller. > > It can work with recent linux kernels 6.x.x. > Two avocado tests was added to check that. > > Unit tests has been made as read/write operations > via mailbox properties. > > Genet integration test is under development. > > Every single commit > 1) builds without errors > 2) passes regression tests > 3) passes style check* > *the only exception is bcm2838-mbox-property-test.c file > containing heavy macros usage which cause a lot of > false-positives of checkpatch.pl. Hi; I had to drop the qtest patches from what I'm going to apply upstream, because it turns out that they don't pass if the host system is big-endian. This is because you read out memory from the guest directly into a host struct: that only works if the host happens to be the same endianness (little) as the guest. One possible way to deal with this would be the following: +/* + * Read and write fields that are in little-endian order. Based on + * the linux-user/qemu.h __put_user_e and __get_user_e macros. + */ +#define put_guest_field(x, hptr) \ + do { \ + (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_le_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_le_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_le_p, abort)))) \ + ((hptr), (x)), (void)0); \ + } while (0) + +#define get_guest_field(x, hptr) \ + do { \ + ((x) = (typeof(*hptr))( \ + __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_le_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_le_p, \ + __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_le_p, abort)))) \ + (hptr)), (void)0); \ + } while (0) + /*----------------------------------------------------------------------------*/ #define DECLARE_TEST_CASE(testname, ...) \ __attribute__((weak)) \ @@ -59,38 +82,41 @@ FIELD(GET_CLOCK_STATE_CMD, NPRES, 1, 1) } mailbox_buffer = { 0 }; \ \ QTestState *qts = qtest_init("-machine raspi4b"); \ + uint32_t req_resp_code, tag_id, size_stat, size, success; \ \ - mailbox_buffer.header.size = sizeof(mailbox_buffer); \ - mailbox_buffer.header.req_resp_code = MBOX_PROCESS_REQUEST; \ + put_guest_field(sizeof(mailbox_buffer), &mailbox_buffer.header.size); \ + put_guest_field(MBOX_PROCESS_REQUEST, \ + &mailbox_buffer.header.req_resp_code); \ \ - mailbox_buffer.tag.id = TEST_TAG(testname); \ - mailbox_buffer.tag.value_buffer_size = MAX( \ + put_guest_field(TEST_TAG(testname), &mailbox_buffer.tag.id); \ + put_guest_field(MAX( \ sizeof(mailbox_buffer.tag.request.value), \ - sizeof(mailbox_buffer.tag.response.value)); \ - mailbox_buffer.tag.request.zero = 0; \ + sizeof(mailbox_buffer.tag.response.value)), \ + &mailbox_buffer.tag.value_buffer_size); \ + put_guest_field(0, &mailbox_buffer.tag.request.zero); \ \ - mailbox_buffer.end_tag = RPI_FWREQ_PROPERTY_END; \ + put_guest_field(RPI_FWREQ_PROPERTY_END, &mailbox_buffer.end_tag); \ \ if (SETUP_FN_NAME(testname, __VA_ARGS__)) { \ SETUP_FN_NAME(testname, __VA_ARGS__)(&mailbox_buffer.tag); \ } \ \ qtest_memwrite(qts, MBOX_TEST_MESSAGE_ADDRESS, \ - &mailbox_buffer, sizeof(mailbox_buffer)); \ + &mailbox_buffer, sizeof(mailbox_buffer)); \ qtest_mbox1_write_message(qts, MBOX_CHANNEL_ID_PROPERTY, \ - MBOX_TEST_MESSAGE_ADDRESS); \ + MBOX_TEST_MESSAGE_ADDRESS); \ \ qtest_mbox0_read_message(qts, MBOX_CHANNEL_ID_PROPERTY, \ - &mailbox_buffer, sizeof(mailbox_buffer)); \ + &mailbox_buffer, sizeof(mailbox_buffer)); \ \ - g_assert_cmphex(mailbox_buffer.header.req_resp_code, ==, MBOX_SUCCESS);\ + get_guest_field(req_resp_code, &mailbox_buffer.header.req_resp_code); \ + g_assert_cmphex(req_resp_code, ==, MBOX_SUCCESS); \ + get_guest_field(tag_id, &mailbox_buffer.tag.id); \ + g_assert_cmphex(tag_id, ==, TEST_TAG(testname)); \ \ - g_assert_cmphex(mailbox_buffer.tag.id, ==, TEST_TAG(testname)); \ - \ - uint32_t size = FIELD_EX32(mailbox_buffer.tag.response.size_stat, \ - MBOX_SIZE_STAT, SIZE); \ - uint32_t success = FIELD_EX32(mailbox_buffer.tag.response.size_stat, \ - MBOX_SIZE_STAT, SUCCESS); \ + get_guest_field(size_stat, &mailbox_buffer.tag.response.size_stat); \ + size = FIELD_EX32(size_stat, MBOX_SIZE_STAT, SIZE); \ + success = FIELD_EX32(size_stat, MBOX_SIZE_STAT, SUCCESS); \ g_assert_cmpint(size, ==, sizeof(mailbox_buffer.tag.response.value)); \ g_assert_cmpint(success, ==, 1); \ \ @@ -110,7 +136,9 @@ FIELD(GET_CLOCK_STATE_CMD, NPRES, 1, 1) /*----------------------------------------------------------------------------*/ DECLARE_TEST_CASE(GET_FIRMWARE_REVISION) { - g_assert_cmpint(tag->response.value.revision, ==, FIRMWARE_REVISION); + uint32_t rev; + get_guest_field(rev, &tag->response.value.revision); + g_assert_cmpint(rev, ==, FIRMWARE_REVISION); } which I have tested works for the one test case that I updated here. (The field comparison part could probably be made less clunky.) Or you could do something else. The test also failed to link on Macos: https://gitlab.com/qemu-project/qemu/-/jobs/6267744541 Undefined symbols for architecture arm64: "_FRAMEBUFFER_GET_ALPHA_MODE__setup", referenced from: _FRAMEBUFFER_GET_ALPHA_MODE__test in bcm2838-mbox-property-test.c.o "_FRAMEBUFFER_GET_DEPTH__setup", referenced from: _FRAMEBUFFER_GET_DEPTH__test in bcm2838-mbox-property-test.c.o (etc) I'm not sure why that is, but I would be suspicious that your use of the __attribute__((weak)) for the setup functions is not portable. thanks -- PMM
On Tue, 27 Feb 2024 at 15:22, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Mon, 26 Feb 2024 at 00:04, Sergey Kambalin <serg.oker@gmail.com> wrote: > > > > Introducing Raspberry Pi 4B model. > > It contains new BCM2838 SoC, PCIE subsystem, > > RNG200, Thermal sensor and Genet network controller. > > > > It can work with recent linux kernels 6.x.x. > > Two avocado tests was added to check that. > > > > Unit tests has been made as read/write operations > > via mailbox properties. > > > > Genet integration test is under development. > > > > Every single commit > > 1) builds without errors > > 2) passes regression tests > > 3) passes style check* > > *the only exception is bcm2838-mbox-property-test.c file > > containing heavy macros usage which cause a lot of > > false-positives of checkpatch.pl. > > Hi; I had to drop the qtest patches from what I'm going to > apply upstream, because it turns out that they don't pass > if the host system is big-endian. The parts of this series that I was able to take are now in upstream git, so if you like you can rebase your series on that. -- PMM