Hi Andre,
On 6/14/21 3:07 PM, Andre Przywara wrote:
> On Thu, 10 Jun 2021 17:38:02 +0100
> Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>
> Hi Alex,
>
>> On 6/10/21 5:13 PM, Andre Przywara wrote:
>>> On Wed, 9 Jun 2021 19:38:10 +0100
>>> Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>>>
>>>> Print a more helpful warning when a MMIO device hasn't set a function to
>>>> generate an FDT instead of causing a segmentation fault by dereferencing a
>>>> NULL pointer.
>>> Not calling generate_mmio_fdt_nodes() if it's NULL is certainly a good
>>> idea, but how did you trigger it?
>> I was able to trigger it when I was hacking a custom MMIO device emulation to test
>> some behaviour in KVM.
>>
>>> Because I am wondering whether every MMIO device needs to have an DT
>>> generator? And if that's not the case, a warning might be already too
>>> much.
>> I don't know how the guest will be able to discover the device if it's not in the
>> DT, that's why I put the warning.
>> If there are devices which can be discovered by
>> the guest when they are missing from the DT, then I'll drop the warning.
> Well, not discovered, probably, but the guest (or parts of the guest,
> think EFI firmware) could have hard-coded knowledge about what to
> expect. That's what we did for the RTC and initially for the CFI flash.
> I agree it's not the best way to do (and we fixed both of those
> devices), but it's also nothing a *user* could do much about, so having a
> pr_debug() there seems more appropriate to me.
I think you're right, pr_debug() makes more sense here.
Thanks,
Alex
>
> Cheers,
> Andre
>
>>> So either just drop a print at all or use pr_info()/pr_debug()?
>>>
>>> Cheers,
>>> Andre
>>>
>>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
>>>> ---
>>>> arm/fdt.c | 7 ++++++-
>>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arm/fdt.c b/arm/fdt.c
>>>> index 02091e9e0bee..06287a13e395 100644
>>>> --- a/arm/fdt.c
>>>> +++ b/arm/fdt.c
>>>> @@ -171,7 +171,12 @@ static int setup_fdt(struct kvm *kvm)
>>>> dev_hdr = device__first_dev(DEVICE_BUS_MMIO);
>>>> while (dev_hdr) {
>>>> generate_mmio_fdt_nodes = dev_hdr->data;
>>>> - generate_mmio_fdt_nodes(fdt, dev_hdr, generate_irq_prop);
>>>> + if (generate_mmio_fdt_nodes) {
>>>> + generate_mmio_fdt_nodes(fdt, dev_hdr, generate_irq_prop);
>>>> + } else {
>>>> + pr_warning("Missing FDT node generator for MMIO device %d",
>>>> + dev_hdr->dev_num);
>>>> + }
>>>> dev_hdr = device__next_dev(dev_hdr);
>>>> }
>>>>
