Re: [PATCH 8/9] xen/ppc: Add stub function and symbol definitions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.08.2023 19:26, Shawn Anastasio wrote:
> On 8/25/23 4:10 AM, Jan Beulich wrote:
>> On 23.08.2023 20:39, Shawn Anastasio wrote:
>>> On 8/8/23 5:27 AM, Jan Beulich wrote:
>>>> On 03.08.2023 01:03, Shawn Anastasio wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/ppc/stubs.c
>>>>> @@ -0,0 +1,351 @@
>>>>> [...]
>>>>> +static void ack_none(struct irq_desc *irq)
>>>>> +{
>>>>> +    BUG();
>>>>> +}
>>>>> +
>>>>> +static void end_none(struct irq_desc *irq)
>>>>> +{
>>>>> +    BUG();
>>>>> +}
>>>>> +
>>>>> +hw_irq_controller no_irq_type = {
>>>>> +    .typename = "none",
>>>>> +    .startup = irq_startup_none,
>>>>> +    .shutdown = irq_shutdown_none,
>>>>> +    .enable = irq_enable_none,
>>>>> +    .disable = irq_disable_none,
>>>>> +    .ack = ack_none,
>>>>> +    .end = end_none
>>>>> +};
>>>>
>>>> I would recommend to avoid filling pointers (and hence having private
>>>> hook functions) where it's not clear whether they'll be required. "end",
>>>> for example, is an optional hook on x86. Iirc common code doesn't use
>>>> any of the hooks.
>>>
>>> Alright, I'll drop the `end_none` stub and leave the .end pointer as
>>> NULL.
>>
>> Yet my comment was about all the (presently dead) hook functions.
> 
> Sorry, I misunderstood. To clarify, by "hook functions" you're referring
> to all of the function pointer fields of hw_irq_controller? Are all
> users of this struct going to properly check for NULL before trying to
> call these function pointers?

If I'm not mistaken, all uses of these hooks are in arch-specific code
(we don't have a proper common layer, unlike Linux). Hence what checks
there are going to be is all up to you. But as long as none of the fields
are used, supplying (stub) hooks is pointless in the first place. IOW you
can leave the pointers at NULL now and still use them later without
checks, so long as at _that_ time you populate them (and then not just
pointing to stub functions).

My main point here is: This introduction of temporary stubs would better
be as little overhead and as little code churn as possible - minimum
number of new symbols/functions/objects, just enough for things to build.

Jan