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

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

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:
>>>> +int map_pages_to_xen(unsigned long virt,
>>>> +                     mfn_t mfn,
>>>> +                     unsigned long nr_mfns,
>>>> +                     unsigned int flags)
>>>
>>> There's a patch in flight regarding the naming of this last parameter.
>>> I guess PPC would best be in sync right away.
>>>
>>
>> I can't seem to find the patch in question and it doesn't seem like it
>> has been merged in the meantime. Could you provide a link?
> 
> Looks like I was misremembering, and it was modify_xen_mappings() instead.
> I'm sorry for the noise.
> 

No problem.

>>>> --- /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?

> Jan

Thanks,
Shawn