On 5/12/2025 2:39 AM, ALOK TIWARI wrote:
On 12-05-2025 04:37, Roman Kisel wrote:
+/*
+ * Not every paravisor supports getting SynIC registers, and
+ * this function may fail. The caller has to make sure that this
function
+ * runs on the CPU of interest.
+ */
Title and Intent: Clearly state the purpose of the function in the first
sentence
/*
* Attempt to get the SynIC register value.
*
* Not all paravisors support reading SynIC registers, so this function
* may fail. The caller must ensure that it is executed on the target
* CPU.
*
* Returns: The SynIC register value or ~0ULL on failure.
* Sets err to -ENODEV if the provided register is not a valid SynIC
* MSR.
*/
+u64 hv_pv_get_synic_register(unsigned int reg, int *err)
+{
+ if (!hv_is_synic_msr(reg)) {
+ *err = -ENODEV;
+ return !0ULL;
+ }
+ return native_read_msr_safe(reg, err);
+}
+EXPORT_SYMBOL_GPL(hv_pv_get_synic_register);
+
+/*
+ * Not every paravisor supports setting SynIC registers, and
+ * this function may fail. The caller has to make sure that this
function
+ * runs on the CPU of interest.
+ */
ditto.
+int hv_pv_set_synic_register(unsigned int reg, u64 val)
+{
+ if (!hv_is_synic_msr(reg))
+ return -ENODEV;
+ return wrmsrl_safe(reg, val);
+}
+EXPORT_SYMBOL_GPL(hv_pv_set_synic_register);
Indeed, I wrote a bit of a novel in the comments which might be
distracting and making it harder to find the point :)
Ought to be more conscious of the reader's perhaps constrained
time budget. I'll restructure that as you suggested!
Thanks,
Alok
--
Thank you,
Roman
