[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/6] hw/arm: new interface for devices which need to behave differently for kernel boot |
Date: |
Fri, 14 Aug 2015 13:48:13 +0100 |
[oops, forgot to update Peter C's email address in the From line;
apologies to everybody else for the duplicate mail.]
On 18 July 2015 at 10:00, Peter Maydell <address@hidden> wrote:
> On 18 July 2015 at 04:55, Peter Crosthwaite
> <address@hidden> wrote:
>> On Thu, Jul 16, 2015 at 1:11 PM, Peter Maydell <address@hidden> wrote:
>>> For ARM we have a little minimalist bootloader in hw/arm/boot.c which
>>> takes the place of firmware if we're directly booting a Linux kernel.
>>> Unfortunately a few devices need special case handling in this situation
>>> to do the initialization which on real hardware would be done by
>>> firmware. (In particular if we're booting a kernel in NonSecure state
>>> then we need to make a TZ-aware GIC put all its interrupts into Group 1,
>>> or the guest will be unable to use them.)
>>>
>>> Create a new QOM interface which can be implemented by devices which
>>> need to do something different from their default reset behaviour.
>>> The callback will be called after machine initialization and before
>>> first reset.
>>> +typedef struct ARMLinuxBootIfClass {
>>> + /*< private >*/
>>> + InterfaceClass parent_class;
>>> +
>>> + /*< public >*/
>>> + /** arm_linux_init: configure the device for a direct boot
>>> + * of an ARM Linux kernel (so that device reset puts it into
>>> + * the state the kernel expects after firmware initialization,
>>> + * rather than the true hardware reset state). This callback is
>>> + * called once after machine construction is complete (before the
>>> + * first system reset).
>>> + *
>>> + * @obj: the object implementing this interface
>>> + * @secure_boot: true if we are booting Secure, false for NonSecure
>>> + * (or for a CPU which doesn't support TrustZone)
>>> + */
>>> + void (*arm_linux_init)(ARMLinuxBootIf *obj, bool secure_boot);
>>
>> Can we drop the "arm_"? ARM is always going to be there in the context
>> as it is in the typename due to ARMLinuxBootIfClass.
>
> Yeah, I guess so. I wasn't really sure what the best method
> name here was.
>
>> So If we are going for an ARM-specific thing, it might make sense to
>> instead drop all the _linux_ stuff and have this call unconditional.
>> Then the API has wider application than just Linux boots. The struct
>> arm_boot_info can be made more widely visible as the one data argument
>> the device accepts, from which security state as well and is_linux can
>> be fished.
>
> I was going to pass arm_boot_info in, but that struct requires
> the target cpu.h so it can't be used in compiled-once objects
> like the GIC code. Hence the single bool parameter.
>
> I'm also not too keen on increasing the set of things we try
> to handle in boot. Currently we do:
> * "firmware", ie the guest code gets to do all setup that it needs,
> just as on hardware
> * "linux kernel", where we provide the more-or-less documented
> boot environment etc for Linux kernels in particular
>
> I think you're implying that we want to support a third thing here?
Any further comment on this? As I said, I'm unconvinced about
making this method more general than we really need. The other
patches have been reviewed so consensus on this API is I think
the only blocker.
thanks
-- PMM