Re: [Xen-devel] [PATCH] x86/HVM: relinquish resources also from hvm_domain_destroy()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.01.2020 18:25, Roger Pau Monné wrote:
> On Tue, Jan 28, 2020 at 04:49:09PM +0100, Jan Beulich wrote:
>> On 28.01.2020 15:54, Roger Pau Monné wrote:
>>> On Tue, Jan 28, 2020 at 02:16:53PM +0100, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/hvm/rtc.c
>>>> +++ b/xen/arch/x86/hvm/rtc.c
>>>> @@ -844,7 +844,8 @@ void rtc_deinit(struct domain *d)
>>>>  {
>>>>      RTCState *s = domain_vrtc(d);
>>>>  
>>>> -    if ( !has_vrtc(d) )
>>>> +    if ( !has_vrtc(d) || !d->arch.hvm.pl_time ||
>>>> +         s->update_timer.status == TIMER_STATUS_invalid )
>>>
>>> You could also check for the port registration AFAICT, which is maybe
>>> more obvious?
>>
>> You called that approach dirty above - I'd like to restrict it
>> to just where no better alternative exists.
> 
> Ack, it didn't seem that bad here because this is a x86 emulated
> device that relies on IO ports, while the ioreq code (albeit x86
> specific ATM) could be used by other arches, and hence would likely
> prefer to avoid using x86 specific details for generic functions, like
> the init or deinit ones.

Likely, but the port I/O handler registration is going to remain
x86-specific, and hence there would pretty certainly also be an
arch-specific init (and may a deinit) function.

>>> I also wonder whether all those time-related emulations could be
>>> grouped into a single helper, that could have a d->arch.hvm.pl_time
>>> instead of having to sprinkle such checks for each device?
>>
>> Quite possible, but not here and not now.
> 
> Sure.
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks. There are two small changes I intend to do, one directly
and one indirectly resulting from Paul's feedback: Also drop
rtc_deinit() from hvm_domain_destroy(). Also drop now pointless
if() from hvm_domain_relinquish_resources(). I'd therefore like
to ask you to confirm the R-b can be left in place, or whether
instead you'd rather wait for v2 to be posted.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel