Re: [Xen-devel] [PATCH v4 10/28] x86/vvtd: Enable Interrupt Remapping through GCMD

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Sun, Feb 11, 2018 at 01:05:01PM +0800, Chao Gao wrote:
> On Fri, Feb 09, 2018 at 05:15:17PM +0000, Roger Pau Monné wrote:
> >On Fri, Nov 17, 2017 at 02:22:17PM +0800, Chao Gao wrote:
> >> +static void write_gcmd_ire(struct vvtd *vvtd, uint32_t val)
> >> +{
> >> +    bool set = val & DMA_GCMD_IRE;
> >> +
> >> +    vvtd_info("%sable Interrupt Remapping\n", set ? "En" : "Dis");
> >> +
> >> +    vvtd->hw.intremap_enabled = set;
> >> +    (set ? vvtd_set_bit : vvtd_clear_bit)
> >> +        (vvtd, DMAR_GSTS_REG, DMA_GSTS_IRES_SHIFT);
> >> +}
> >> +
> >>  static void write_gcmd_sirtp(struct vvtd *vvtd, uint32_t val)
> >>  {
> >>      uint64_t irta = vvtd_get_reg_quad(vvtd, DMAR_IRTA_REG);
> >> @@ -131,16 +205,29 @@ static void write_gcmd_sirtp(struct vvtd *vvtd, 
> >> uint32_t val)
> >>       * the 'Set Interrupt Remap Table Pointer' operation.
> >>       */
> >>      vvtd_clear_bit(vvtd, DMAR_GSTS_REG, DMA_GSTS_SIRTPS_SHIFT);
> >> +    if ( vvtd->hw.intremap_enabled )
> >> +        vvtd_info("Update Interrupt Remapping Table when active\n");
> >>  
> >>      if ( gfn_x(vvtd->hw.irt) != PFN_DOWN(DMA_IRTA_ADDR(irta)) ||
> >>           vvtd->hw.irt_max_entry != DMA_IRTA_SIZE(irta) )
> >>      {
> >> +        if ( vvtd->irt_base )
> >> +        {
> >> +            unmap_guest_pages(vvtd->irt_base,
> >> +                              PFN_UP(vvtd->hw.irt_max_entry *
> >> +                                     sizeof(struct iremap_entry)));
> >> +            vvtd->irt_base = NULL;
> >> +        }
> >
> >Shouldn't this be done when sirtp is switched off, instead of when
> >it's updated?
> >
> >What happens in the following scenario:
> >
> >- Guest writes gfn to irta.
> >- Guest enables sirtps.
> >- Guest disables sirtps.
> 
> Disabling SIRTP isn't clear to me. Maybe you mean writing to GCMD with
> SIRTP cleared. Hardware ignores write 0 to SIRTP I think becasue SIRTP
> is a one-shot bit. Please refer to the example in VT-d spec 10.4.4.
> Each time IRTP is updated, the old mapping should be destroyed and the
> new mapping should be created.

After reading the spec I agree, there's no such thing as clearing
SIRTP.

You should however unmap the IRTA address when IRE is cleared
(interrupt remapping disabled), which AFAICT you don't to do now.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel