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 <andre.przywara@xxxxxxx>
>
> 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
>> kvmarm@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
