>>> On 15.05.18 at 13:43, <wei.l...@citrix.com> wrote:
> On Thu, May 10, 2018 at 06:15:04PM +0100, Roger Pau Monne wrote:
>> @@ -1014,6 +1034,30 @@ static int vcpu_hvm(struct xc_dom_image *dom)
>> if ( dom->start_info_seg.pfn )
>> bsp_ctx.cpu.rbx = dom->start_info_seg.pfn << PAGE_SHIFT;
>>
>> + /* Set the MTRR. */
>> + bsp_ctx.mtrr_d.typecode = HVM_SAVE_CODE(MTRR);
>> + bsp_ctx.mtrr_d.instance = 0;
>> + bsp_ctx.mtrr_d.length = HVM_SAVE_LENGTH(MTRR);
>> +
>> + mtrr_record = hvm_get_save_record(full_ctx, HVM_SAVE_CODE(MTRR), 0);
>> + if ( !mtrr_record )
>> + {
>> + xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
>> + "%s: unable to get MTRR save record", __func__);
>> + goto out;
>> + }
>> +
>> + memcpy(&bsp_ctx.mtrr, mtrr_record, sizeof(bsp_ctx.mtrr));
>> +
>> + /* TODO: maybe this should be a firmware option instead? */
>> + if ( !dom->device_model )
>> + /*
>> + * Enable MTRR, set default type to WB.
>> + * TODO: add MMIO areas as UC when passthrough is supported.
>> + */
>> + bsp_ctx.mtrr.msr_mtrr_def_type = MTRR_TYPE_WRBACK |
>> + MTRR_DEF_TYPE_ENABLE;
>> +
>
> Hrm... I'm not entirely happy with this in toolstack code but there
> doesn't seem to be a better way to do this at the moment, considering
> hypervisor doesn't distinguish HVM and PVH guests.
>
> Anyway, the code looks correct to me. I would rather see something in
> hypervisor to deal with this, but I won't object to this either.
But doing it in the hypervisor would be a layering violation imo: The
hypervisor should set MTRR state to power-on / reset defaults, which
it does. It's firmware which is supposed to adapt their values to actual
system characteristics (RAM and MMIO ranges), and libxc has to play
the role of firmware here short of there being any in the guest itself.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
