On 14/05/2020 13:20, Mark Rutland wrote: Hi,
> > On Thu, May 14, 2020 at 10:45:53AM +0100, Andre Przywara wrote: >> On arm and arm64 we expose the Motorola RTC emulation to the guest, >> but never advertised this in the device tree. >> >> EDK-2 seems to rely on this device, but on its hardcoded address. To >> make this more future-proof, add a DT node with the address in it. >> EDK-2 can then read the proper address from there, and we can change >> this address later (with the flexible memory layout). >> >> Please note that an arm64 Linux kernel is not ready to use this device, >> there are some include files missing under arch/arm64 to compile the >> driver. I hacked this up in the kernel, just to verify this DT snippet >> is correct, but don't see much value in enabling this properly in >> Linux. >> >> Signed-off-by: Andre Przywara <[email protected]> > > With EFI at least, the expectation is that the RTC is accesses via the > runtime EFI services. So as long as EFI knows about the RTC and the > kernel knows about EFI, the kernel can use the RTC that way. Yes, this is how it works at the moment. > It would be > problematic were the kernel to mess with the RTC behind the back of EFI > or vice-versa, so it doesn't make sense to expose voth view to the > kernel simultaneously. Agreed. > I don't think it makes sense to expose this in the DT unless EFI were > also clearing this from the DT before handing that on to Linux. If we > have that, I think it'd be fine, but on its own this patch introduces a > potnetial problem that I think we should avoid. Yes, removing or at least disabling the node was the plan, but first we need to convince EDK-2 to actually use it first ;-) At the moment the addresses are hardcoded, and things look fine, but we want (and need to) become more flexible with the memory map, so just relying on 0x70/0x71 doesn't have a future. Especially this low memory areas already has problems. >From a kvmtool's perspective this "two users problem" shouldn't matter, though, that's a problem that EDK-2 needs to solve, if it chooses to use the RTC in its runtime. And as mentioned: right now Linux can't use the Motorola RTC driver on arm64, so there is no danger atm that the kernel picks the device up and uses it. But I would rather sooner than later let EDK-2 use the DT address, before this hardcoded address usage becomes to widespread to be ignored. And yes, I will push for disabling the DT node then in EDK-2. Cheers, Andre. >> --- >> hw/rtc.c | 44 ++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 38 insertions(+), 6 deletions(-) >> >> diff --git a/hw/rtc.c b/hw/rtc.c >> index c1fa72f2..5483879f 100644 >> --- a/hw/rtc.c >> +++ b/hw/rtc.c >> @@ -130,24 +130,56 @@ static struct ioport_operations >> cmos_ram_index_ioport_ops = { >> .io_out = cmos_ram_index_out, >> }; >> >> +#ifdef CONFIG_HAS_LIBFDT >> +static void generate_rtc_fdt_node(void *fdt, >> + struct device_header *dev_hdr, >> + void (*generate_irq_prop)(void *fdt, >> + u8 irq, >> + enum irq_type)) >> +{ >> + u64 reg_prop[2] = { cpu_to_fdt64(0x70), cpu_to_fdt64(2) }; >> + >> + _FDT(fdt_begin_node(fdt, "rtc")); >> + _FDT(fdt_property_string(fdt, "compatible", "motorola,mc146818")); >> + _FDT(fdt_property(fdt, "reg", reg_prop, sizeof(reg_prop))); >> + _FDT(fdt_end_node(fdt)); >> +} >> +#else >> +#define generate_rtc_fdt_node NULL >> +#endif >> + >> +struct device_header rtc_dev_hdr = { >> + .bus_type = DEVICE_BUS_IOPORT, >> + .data = generate_rtc_fdt_node, >> +}; >> + >> int rtc__init(struct kvm *kvm) >> { >> - int r = 0; >> + int r; >> + >> + r = device__register(&rtc_dev_hdr); >> + if (r < 0) >> + return r; >> >> /* PORT 0070-007F - CMOS RAM/RTC (REAL TIME CLOCK) */ >> r = ioport__register(kvm, 0x0070, &cmos_ram_index_ioport_ops, 1, NULL); >> if (r < 0) >> - return r; >> + goto out_device; >> >> r = ioport__register(kvm, 0x0071, &cmos_ram_data_ioport_ops, 1, NULL); >> - if (r < 0) { >> - ioport__unregister(kvm, 0x0071); >> - return r; >> - } >> + if (r < 0) >> + goto out_ioport; >> >> /* Set the VRT bit in Register D to indicate valid RAM and time */ >> rtc.cmos_data[RTC_REG_D] = RTC_REG_D_VRT; >> >> + return r; >> + >> +out_ioport: >> + ioport__unregister(kvm, 0x0070); >> +out_device: >> + device__unregister(&rtc_dev_hdr); >> + >> return r; >> } >> dev_init(rtc__init); >> -- >> 2.17.1 >> >> _______________________________________________ >> kvmarm mailing list >> [email protected] >> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
