| Message ID | 1502310866-10450-26-git-send-email-tianyu.lan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 09, 2017 at 04:34:26PM -0400, Lan Tianyu wrote: > From: Chao Gao <chao.gao@intel.com> > > 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@intel.com> > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > --- > xen/drivers/passthrough/vtd/vvtd.c | 98 ++++++++++++++++++++++------------ > xen/include/public/arch-x86/hvm/save.h | 24 ++++++++- > 2 files changed, 88 insertions(+), 34 deletions(-) > > diff --git a/xen/drivers/passthrough/vtd/vvtd.c b/xen/drivers/passthrough/vtd/vvtd.c > index 4f5e28e..dd6be83 100644 > --- a/xen/drivers/passthrough/vtd/vvtd.c > +++ b/xen/drivers/passthrough/vtd/vvtd.c > @@ -20,6 +20,7 @@ > > #include <xen/domain_page.h> > #include <xen/lib.h> > +#include <xen/hvm/save.h> > #include <xen/sched.h> > #include <xen/types.h> > #include <xen/viommu.h> > @@ -32,39 +33,26 @@ > #include <asm/page.h> > #include <asm/p2m.h> > #include <asm/system.h> > +#include <public/hvm/save.h> > > #include "iommu.h" > #include "vtd.h" > > -struct hvm_hw_vvtd_regs { > - uint8_t data[1024]; > -}; > - > /* Status field of struct vvtd */ > #define VIOMMU_STATUS_DEFAULT (0) > #define VIOMMU_STATUS_IRQ_REMAPPING_ENABLED (1 << 0) > #define VIOMMU_STATUS_DMA_REMAPPING_ENABLED (1 << 1) > > #define vvtd_irq_remapping_enabled(vvtd) \ > - (vvtd->status & VIOMMU_STATUS_IRQ_REMAPPING_ENABLED) > + (vvtd->hw.status & VIOMMU_STATUS_IRQ_REMAPPING_ENABLED) > > struct vvtd { > - /* VIOMMU_STATUS_XXX */ > - int status; > - /* Fault Recording index */ > - int frcd_idx; > /* Address range of remapping hardware register-set */ > uint64_t base_addr; > uint64_t length; > /* Point back to the owner domain */ > struct domain *domain; > - /* Is in Extended Interrupt Mode? */ > - bool eim; > - /* Max remapping entries in IRT */ > - int irt_max_entry; > - /* Interrupt remapping table base gfn */ > - uint64_t irt; > - > + struct hvm_hw_vvtd hw; This should have been done in the first patch IMHO, rather than moving things around now. Directly define hvm_hw_vvtd instead of itroducing it now. > struct hvm_hw_vvtd_regs *regs; > struct page_info *regs_page; > }; > @@ -370,12 +358,12 @@ static int vvtd_alloc_frcd(struct vvtd *vvtd) > int prev; > > /* Set the F bit to indicate the FRCD is in use. */ > - if ( vvtd_test_and_set_bit(vvtd, DMA_FRCD(vvtd->frcd_idx, DMA_FRCD3_OFFSET), > + if ( vvtd_test_and_set_bit(vvtd, DMA_FRCD(vvtd->hw.frcd_idx, DMA_FRCD3_OFFSET), > DMA_FRCD_F_BIT) ) > { > - prev = vvtd->frcd_idx; > - vvtd->frcd_idx = (prev + 1) % DMA_FRCD_REG_NR; > - return vvtd->frcd_idx; > + prev = vvtd->hw.frcd_idx; > + vvtd->hw.frcd_idx = (prev + 1) % DMA_FRCD_REG_NR; > + return vvtd->hw.frcd_idx; > } > return -1; > } > @@ -712,12 +700,12 @@ static int vvtd_handle_gcmd_ire(struct vvtd *vvtd, uint32_t val) > > if ( val & DMA_GCMD_IRE ) > { > - vvtd->status |= VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; > + vvtd->hw.status |= VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; > __vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_IRES_BIT); > } > else > { > - vvtd->status |= ~VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; > + vvtd->hw.status |= ~VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; > __vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_IRES_BIT); > } > > @@ -736,11 +724,11 @@ static int vvtd_handle_gcmd_sirtp(struct vvtd *vvtd, uint32_t val) > "active." ); > > vvtd_get_reg_quad(vvtd, DMAR_IRTA_REG, irta); > - vvtd->irt = DMA_IRTA_ADDR(irta) >> PAGE_SHIFT; > - vvtd->irt_max_entry = DMA_IRTA_SIZE(irta); > - vvtd->eim = DMA_IRTA_EIME(irta); > + vvtd->hw.irt = DMA_IRTA_ADDR(irta) >> PAGE_SHIFT; > + vvtd->hw.irt_max_entry = DMA_IRTA_SIZE(irta); > + vvtd->hw.eim = DMA_IRTA_EIME(irta); > VVTD_DEBUG(VVTD_DBG_RW, "Update IR info (addr=%lx eim=%d size=%d).", > - vvtd->irt, vvtd->eim, vvtd->irt_max_entry); > + vvtd->hw.irt, vvtd->hw.eim, vvtd->hw.irt_max_entry); > __vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_SIRTPS_BIT); > > return X86EMUL_OKAY; > @@ -947,13 +935,13 @@ static int vvtd_get_entry(struct vvtd *vvtd, > > VVTD_DEBUG(VVTD_DBG_TRANS, "interpret a request with index %x", entry); > > - if ( entry > vvtd->irt_max_entry ) > + if ( entry > vvtd->hw.irt_max_entry ) > { > ret = VTD_FR_IR_INDEX_OVER; > goto handle_fault; > } > > - ret = map_guest_page(vvtd->domain, vvtd->irt + (entry >> IREMAP_ENTRY_ORDER), > + ret = map_guest_page(vvtd->domain, vvtd->hw.irt + (entry >> IREMAP_ENTRY_ORDER), Line length. > (void**)&irt_page); > if ( ret ) > { > @@ -1077,6 +1065,49 @@ static int vvtd_get_irq_info(struct domain *d, > return 0; > } > > +static int vvtd_load_regs(struct domain *d, hvm_domain_context_t *h) > +{ > + if ( !domain_vvtd(d) ) > + return -ENODEV; > + > + if ( hvm_load_entry(IOMMU_REGS, h, domain_vvtd(d)->regs) ) > + return -EINVAL; > + > + return 0; > +} > + > +static int vvtd_save_regs(struct domain *d, hvm_domain_context_t *h) > +{ > + if ( !domain_vvtd(d) ) > + return 0; > + > + return hvm_save_entry(IOMMU_REGS, 0, h, domain_vvtd(d)->regs); > +} > + > +static int vvtd_load_hidden(struct domain *d, hvm_domain_context_t *h) > +{ > + if ( !domain_vvtd(d) ) > + return -ENODEV; > + > + if ( hvm_load_entry(IOMMU, h, &domain_vvtd(d)->hw) ) > + return -EINVAL; > + > + return 0; > +} > + > +static int vvtd_save_hidden(struct domain *d, hvm_domain_context_t *h) > +{ > + if ( !domain_vvtd(d) ) > + return 0; > + > + return hvm_save_entry(IOMMU, 0, h, &domain_vvtd(d)->hw); > +} > + > +HVM_REGISTER_SAVE_RESTORE(IOMMU, vvtd_save_hidden, vvtd_load_hidden, > + 1, HVMSR_PER_DOM); > +HVM_REGISTER_SAVE_RESTORE(IOMMU_REGS, vvtd_save_regs, vvtd_load_regs, > + 1, HVMSR_PER_DOM); > + > static void vvtd_reset(struct vvtd *vvtd, uint64_t capability) > { > uint64_t cap = DMA_CAP_NFR | DMA_CAP_SLLPS | DMA_CAP_FRO | > @@ -1122,12 +1153,13 @@ static int vvtd_create(struct domain *d, struct viommu *viommu) > vvtd->base_addr = viommu->base_address; > vvtd->length = viommu->length; > vvtd->domain = d; > - vvtd->status = VIOMMU_STATUS_DEFAULT; > - vvtd->eim = 0; > - vvtd->irt = 0; > - vvtd->irt_max_entry = 0; > - vvtd->frcd_idx = 0; > + vvtd->hw.status = VIOMMU_STATUS_DEFAULT; > + vvtd->hw.eim = 0; > + vvtd->hw.irt = 0; > + vvtd->hw.irt_max_entry = 0; > + vvtd->hw.frcd_idx = 0; > register_mmio_handler(d, &vvtd_mmio_ops); > + viommu->priv = (void *)vvtd; > return 0; > > out2: > 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. Roger.
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@intel.com> >> >> 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@intel.com> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >> --- >> 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. Thanks Chao
On Fri, Aug 25, 2017 at 03:00:32AM -0600, Jan Beulich wrote: >>>> On 25.08.17 at 08:35, <chao.gao@intel.com> 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@intel.com> >>>> >>>> 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@intel.com> >>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >>>> --- >>>> 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. It seems likely that they can be combined. will give it a try. Thanks Chao
>>> On 25.08.17 at 08:35, <chao.gao@intel.com> 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@intel.com> >>> >>> 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@intel.com> >>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >>> --- >>> 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
diff --git a/xen/drivers/passthrough/vtd/vvtd.c b/xen/drivers/passthrough/vtd/vvtd.c index 4f5e28e..dd6be83 100644 --- a/xen/drivers/passthrough/vtd/vvtd.c +++ b/xen/drivers/passthrough/vtd/vvtd.c @@ -20,6 +20,7 @@ #include <xen/domain_page.h> #include <xen/lib.h> +#include <xen/hvm/save.h> #include <xen/sched.h> #include <xen/types.h> #include <xen/viommu.h> @@ -32,39 +33,26 @@ #include <asm/page.h> #include <asm/p2m.h> #include <asm/system.h> +#include <public/hvm/save.h> #include "iommu.h" #include "vtd.h" -struct hvm_hw_vvtd_regs { - uint8_t data[1024]; -}; - /* Status field of struct vvtd */ #define VIOMMU_STATUS_DEFAULT (0) #define VIOMMU_STATUS_IRQ_REMAPPING_ENABLED (1 << 0) #define VIOMMU_STATUS_DMA_REMAPPING_ENABLED (1 << 1) #define vvtd_irq_remapping_enabled(vvtd) \ - (vvtd->status & VIOMMU_STATUS_IRQ_REMAPPING_ENABLED) + (vvtd->hw.status & VIOMMU_STATUS_IRQ_REMAPPING_ENABLED) struct vvtd { - /* VIOMMU_STATUS_XXX */ - int status; - /* Fault Recording index */ - int frcd_idx; /* Address range of remapping hardware register-set */ uint64_t base_addr; uint64_t length; /* Point back to the owner domain */ struct domain *domain; - /* Is in Extended Interrupt Mode? */ - bool eim; - /* Max remapping entries in IRT */ - int irt_max_entry; - /* Interrupt remapping table base gfn */ - uint64_t irt; - + struct hvm_hw_vvtd hw; struct hvm_hw_vvtd_regs *regs; struct page_info *regs_page; }; @@ -370,12 +358,12 @@ static int vvtd_alloc_frcd(struct vvtd *vvtd) int prev; /* Set the F bit to indicate the FRCD is in use. */ - if ( vvtd_test_and_set_bit(vvtd, DMA_FRCD(vvtd->frcd_idx, DMA_FRCD3_OFFSET), + if ( vvtd_test_and_set_bit(vvtd, DMA_FRCD(vvtd->hw.frcd_idx, DMA_FRCD3_OFFSET), DMA_FRCD_F_BIT) ) { - prev = vvtd->frcd_idx; - vvtd->frcd_idx = (prev + 1) % DMA_FRCD_REG_NR; - return vvtd->frcd_idx; + prev = vvtd->hw.frcd_idx; + vvtd->hw.frcd_idx = (prev + 1) % DMA_FRCD_REG_NR; + return vvtd->hw.frcd_idx; } return -1; } @@ -712,12 +700,12 @@ static int vvtd_handle_gcmd_ire(struct vvtd *vvtd, uint32_t val) if ( val & DMA_GCMD_IRE ) { - vvtd->status |= VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; + vvtd->hw.status |= VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; __vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_IRES_BIT); } else { - vvtd->status |= ~VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; + vvtd->hw.status |= ~VIOMMU_STATUS_IRQ_REMAPPING_ENABLED; __vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_IRES_BIT); } @@ -736,11 +724,11 @@ static int vvtd_handle_gcmd_sirtp(struct vvtd *vvtd, uint32_t val) "active." ); vvtd_get_reg_quad(vvtd, DMAR_IRTA_REG, irta); - vvtd->irt = DMA_IRTA_ADDR(irta) >> PAGE_SHIFT; - vvtd->irt_max_entry = DMA_IRTA_SIZE(irta); - vvtd->eim = DMA_IRTA_EIME(irta); + vvtd->hw.irt = DMA_IRTA_ADDR(irta) >> PAGE_SHIFT; + vvtd->hw.irt_max_entry = DMA_IRTA_SIZE(irta); + vvtd->hw.eim = DMA_IRTA_EIME(irta); VVTD_DEBUG(VVTD_DBG_RW, "Update IR info (addr=%lx eim=%d size=%d).", - vvtd->irt, vvtd->eim, vvtd->irt_max_entry); + vvtd->hw.irt, vvtd->hw.eim, vvtd->hw.irt_max_entry); __vvtd_set_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_SIRTPS_BIT); return X86EMUL_OKAY; @@ -947,13 +935,13 @@ static int vvtd_get_entry(struct vvtd *vvtd, VVTD_DEBUG(VVTD_DBG_TRANS, "interpret a request with index %x", entry); - if ( entry > vvtd->irt_max_entry ) + if ( entry > vvtd->hw.irt_max_entry ) { ret = VTD_FR_IR_INDEX_OVER; goto handle_fault; } - ret = map_guest_page(vvtd->domain, vvtd->irt + (entry >> IREMAP_ENTRY_ORDER), + ret = map_guest_page(vvtd->domain, vvtd->hw.irt + (entry >> IREMAP_ENTRY_ORDER), (void**)&irt_page); if ( ret ) { @@ -1077,6 +1065,49 @@ static int vvtd_get_irq_info(struct domain *d, return 0; } +static int vvtd_load_regs(struct domain *d, hvm_domain_context_t *h) +{ + if ( !domain_vvtd(d) ) + return -ENODEV; + + if ( hvm_load_entry(IOMMU_REGS, h, domain_vvtd(d)->regs) ) + return -EINVAL; + + return 0; +} + +static int vvtd_save_regs(struct domain *d, hvm_domain_context_t *h) +{ + if ( !domain_vvtd(d) ) + return 0; + + return hvm_save_entry(IOMMU_REGS, 0, h, domain_vvtd(d)->regs); +} + +static int vvtd_load_hidden(struct domain *d, hvm_domain_context_t *h) +{ + if ( !domain_vvtd(d) ) + return -ENODEV; + + if ( hvm_load_entry(IOMMU, h, &domain_vvtd(d)->hw) ) + return -EINVAL; + + return 0; +} + +static int vvtd_save_hidden(struct domain *d, hvm_domain_context_t *h) +{ + if ( !domain_vvtd(d) ) + return 0; + + return hvm_save_entry(IOMMU, 0, h, &domain_vvtd(d)->hw); +} + +HVM_REGISTER_SAVE_RESTORE(IOMMU, vvtd_save_hidden, vvtd_load_hidden, + 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE(IOMMU_REGS, vvtd_save_regs, vvtd_load_regs, + 1, HVMSR_PER_DOM); + static void vvtd_reset(struct vvtd *vvtd, uint64_t capability) { uint64_t cap = DMA_CAP_NFR | DMA_CAP_SLLPS | DMA_CAP_FRO | @@ -1122,12 +1153,13 @@ static int vvtd_create(struct domain *d, struct viommu *viommu) vvtd->base_addr = viommu->base_address; vvtd->length = viommu->length; vvtd->domain = d; - vvtd->status = VIOMMU_STATUS_DEFAULT; - vvtd->eim = 0; - vvtd->irt = 0; - vvtd->irt_max_entry = 0; - vvtd->frcd_idx = 0; + vvtd->hw.status = VIOMMU_STATUS_DEFAULT; + vvtd->hw.eim = 0; + vvtd->hw.irt = 0; + vvtd->hw.irt_max_entry = 0; + vvtd->hw.frcd_idx = 0; register_mmio_handler(d, &vvtd_mmio_ops); + viommu->priv = (void *)vvtd; return 0; out2: 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); + /* * Largest type-code in use */ -#define HVM_SAVE_CODE_MAX 20 +#define HVM_SAVE_CODE_MAX 22 #endif /* __XEN_PUBLIC_HVM_SAVE_X86_H__ */