Re: [Xen-devel] [PATCH V2 25/25] x86/vvtd: save and restore emulated VT-d

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.08.17 at 08:35, <chao.gao@xxxxxxxxx> wrote:
> On Wed, Aug 23, 2017 at 01:19:41PM +0100, Roger Pau Monné wrote:
>>On Wed, Aug 09, 2017 at 04:34:26PM -0400, Lan Tianyu wrote:
>>> From: Chao Gao <chao.gao@xxxxxxxxx>
>>> 
>>> Wrap some useful status in a new structure hvm_hw_vvtd, following
>>> the customs of vlapic, vioapic and etc. Provide two save-restore
>>> pairs to save/restore registers and non-register status.
>>> 
>>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>>> ---
>>> diff --git a/xen/include/public/arch-x86/hvm/save.h 
> b/xen/include/public/arch-x86/hvm/save.h
>>> index fd7bf3f..10536cb 100644
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -639,10 +639,32 @@ struct hvm_msr {
>>>  
>>>  #define CPU_MSR_CODE  20
>>>  
>>> +struct hvm_hw_vvtd_regs {
>>> +    uint8_t data[1024];
>>> +};
>>> +
>>> +DECLARE_HVM_SAVE_TYPE(IOMMU_REGS, 21, struct hvm_hw_vvtd_regs);
>>> +
>>> +struct hvm_hw_vvtd
>>> +{
>>> +    /* VIOMMU_STATUS_XXX */
>>> +    uint32_t status;
>>> +    /* Fault Recording index */
>>> +    uint32_t frcd_idx;
>>> +    /* Is in Extended Interrupt Mode? */
>>> +    uint32_t eim;
>>> +    /* Max remapping entries in IRT */
>>> +    uint32_t irt_max_entry;
>>> +    /* Interrupt remapping table base gfn */
>>> +    uint64_t irt;
>>> +};
>>> +
>>> +DECLARE_HVM_SAVE_TYPE(IOMMU, 22, struct hvm_hw_vvtd);
>>
>>Why two separate structures? It should be the same structure.
> 
> Hi, Roger.
> 
> Thank you for your review. I agree with most of your comments on the
> whole series. I will only reply to some points I think still need
> discussion.
> 
> Here we use two separate structures for some field cannot be infered
> from the struct hvm_hw_vvtd_regs. For example, the 'irt' is the gfn of
> the base address Interrupt Remapping Table. The field is set through
> 1. set the register DMAR_IRTE_REG in hvm_hw_vvtd_regs.
> 2. send a command to vtd by writting another command register.
> 
> If the current base address is A, and guest wants to update the base
> address to B and finish the first step. Unfortunately, saving and
> restoring happen here. In this case, we need the struct hvm_hw_vvtd
> to correctly restore some information.

Hmm, the way I've understood Roger's question is why you
don't combine the two structures into one, not whether one
of the two can be omitted.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel