Re: [PATCH for-4.22 3/5] x86/vRTC: support century field

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.05.2026 10:42, Roger Pau Monné wrote:
> On Fri, May 15, 2026 at 08:52:03AM +0200, Jan Beulich wrote:
>> On 13.05.2026 21:40, Roger Pau Monné wrote:
>>> On Wed, May 13, 2026 at 05:24:13PM +0200, Jan Beulich wrote:
>>>> On 13.05.2026 17:14, Roger Pau Monné wrote:
>>>>> On Wed, May 13, 2026 at 04:58:57PM +0200, Jan Beulich wrote:
>>>>>> On 13.05.2026 16:24, Roger Pau Monné wrote:
>>>>>>> On Tue, May 12, 2026 at 04:59:35PM +0200, Jan Beulich wrote:
>>>>>>>> @@ -515,7 +538,10 @@ static int rtc_ioport_write(void *opaque
>>>>>>>>              /* Fetch the current time and update just this field. */
>>>>>>>>              s->current_tm = gmtime(get_localtime(d));
>>>>>>>>              rtc_copy_date(s);
>>>>>>>> -            s->hw.cmos_data[s->hw.cmos_index] = data;
>>>>>>>> +            if ( s->hw.cmos_index != RTC_CENTURY )
>>>>>>>> +                s->hw.cmos_data[s->hw.cmos_index] = data;
>>>>>>>> +            else
>>>>>>>> +                s->hw.century = data;
>>>>>>>>              rtc_set_time(s);
>>>>>>>>          }
>>>>>>>>          alarm_timer_update(s);
>>>>>>>
>>>>>>> Don't you need to adjust the tail return of rtc_ioport_write() (below
>>>>>>> the context here) to return 0 when s->hw.cmos_index == RTC_CENTURY, so
>>>>>>> the set value is also propagated to the DM, and not only the index?
>>>>>>
>>>>>> I don't think so. The case of us not handling RTC_CENTURY is dealt with
>>>>>> earlier in the function. Whereas when we handle the field, we don't want
>>>>>> to forward (like for all the other RTC fields).
>>>>>
>>>>> Right, so then you also want to adjust the top part of
>>>>> rtc_ioport_write() to not propagate the write to the 0x70 IO port when
>>>>> data is RTC_CENTURY?  Otherwise you propagate the write to port 0x70,
>>>>> but not the read/write to port 0x71?
>>>>
>>>> I can't, as whether to forward depends on the data subsequently written.
>>>> Propagating the index "just in case" is the only workable model that I
>>>> can think of. And as guests can do any number of successive port 70
>>>> writes, the DM needs to cope with this anyway.
>>>
>>> Hm, I see, the newly written value to port 0x71 is taken into account
>>> to decide whether to forward to the DM or not.
>>>
>>> I wonder: would it be simpler to extend the size of the hvm_hw_rtc
>>> structure so that Xen can detect whether the incoming VM has support
>>> for the century field, and then avoid having to play heuristics
>>> with the value itself?
>>
>> It would surely be possible (and we may need to do so anyway for the alarm
>> date/month fields), but I wanted to get away without doing so here. And it
>> seemed pretty reasonable to leverage the padding field for this.
>>
>>>  We would know ahead of starting the guest
>>> whether RTC_CENTURY is supposed to be handled by Xen or forwarded to
>>> the DM, and we won't be limited to use 0 as the sentinel value for not
>>> exposing RTC_CENTURY.
>>
>> These are the positive aspects. The negative one is that "backwards"
>> migration would break with the larger record size. Whereas with the padding
>> field used, it won't: The guest likely won't notice that the value at 0x32
>> isn't updated anymore, i.e. stays at value 20.
> 
> Sorry, maybe I'm confused, but when migrating backwards the access
> won't be handled by Xen anymore, and hence we don't know what it would
> return, it's up to the catch-all device model.

Oh, you're right of course. The value returned would be whatever the DM
deems appropriate.

Jan