Re: [Xen-devel] [PATCH v2 22/30] xen/x86: support PVHv2 Dom0 BAR remapping

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Oct 03, 2016 at 11:10:44AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne [mailto:roger.pau@xxxxxxxxxx]
> > Sent: 27 September 2016 16:57
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: konrad.wilk@xxxxxxxxxx; boris.ostrovsky@xxxxxxxxxx; Roger Pau Monne
> > <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jan
> > Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> > <Andrew.Cooper3@xxxxxxxxxx>
> > Subject: [PATCH v2 22/30] xen/x86: support PVHv2 Dom0 BAR remapping
> > 
> > Add handlers to detect attemps from a PVHv2 Dom0 to change the position
> > of the PCI BARs and properly remap them.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/io.c         |   2 +
> >  xen/drivers/passthrough/pci.c | 307
> > ++++++++++++++++++++++++++++++++++++++++++
> >  xen/include/asm-x86/hvm/io.h  |  19 +++
> >  xen/include/xen/pci.h         |   3 +
> >  4 files changed, 331 insertions(+)
> > 
> > diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index
> > 7de1de3..4db0266 100644
> > --- a/xen/arch/x86/hvm/io.c
> > +++ b/xen/arch/x86/hvm/io.c
> > @@ -862,6 +862,8 @@ static int hvm_pt_add_register(struct hvm_pt_device
> > *dev,  }
> > 
> >  static struct hvm_pt_handler_init *hwdom_pt_handlers[] = {
> > +    &hvm_pt_bar_init,
> > +    &hvm_pt_vf_bar_init,
> >  };
> > 
> >  int hwdom_add_device(struct pci_dev *pdev) diff --git
> > a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index
> > 6d831dd..60c9e74 100644
> > --- a/xen/drivers/passthrough/pci.c
> > +++ b/xen/drivers/passthrough/pci.c
> > @@ -633,6 +633,313 @@ static int pci_size_bar(unsigned int seg, unsigned
> > int bus, unsigned int slot,
> >      return 0;
> >  }
> > 
> > +static bool bar_reg_is_vf(uint32_t real_offset, uint32_t
> > +handler_offset) {
> > +    if ( real_offset - handler_offset == PCI_SRIOV_BAR )
> > +        return true;
> > +    else
> > +        return false;
> > +}
> > +
> 
> Return the bool expression rather than the if-then-else?

Done.
 
> > +static int bar_reg_init(struct hvm_pt_device *s,
> > +                        struct hvm_pt_reg_handler *handler,
> > +                        uint32_t real_offset, uint32_t *data) {
> > +    uint8_t seg, bus, slot, func;
> > +    uint64_t addr, size;
> > +    uint32_t val;
> > +    unsigned int index = handler->offset / 4;
> > +    bool vf = bar_reg_is_vf(real_offset, handler->offset);
> > +    struct hvm_pt_bar *bars = (vf ? s->vf_bars : s->bars);
> > +    int num_bars = (vf ? PCI_SRIOV_NUM_BARS : s->num_bars);
> > +    int rc;
> > +
> > +    if ( index >= num_bars )
> > +    {
> > +        *data = HVM_PT_INVALID_REG;
> > +        return 0;
> > +    }
> > +
> > +    seg = s->pdev->seg;
> > +    bus = s->pdev->bus;
> > +    slot = PCI_SLOT(s->pdev->devfn);
> > +    func = PCI_FUNC(s->pdev->devfn);
> > +    val = pci_conf_read32(seg, bus, slot, func, real_offset);
> > +
> > +    if ( index > 0 && bars[index - 1].type == HVM_PT_BAR_MEM64_LO )
> > +        bars[index].type = HVM_PT_BAR_MEM64_HI;
> > +    else if ( (val & PCI_BASE_ADDRESS_SPACE) ==
> > PCI_BASE_ADDRESS_SPACE_IO )
> > +    {
> > +        bars[index].type = HVM_PT_BAR_UNUSED;
> > +    }
> > +    else if ( (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK) ==
> > +              PCI_BASE_ADDRESS_MEM_TYPE_64 )
> > +        bars[index].type = HVM_PT_BAR_MEM64_LO;
> > +    else
> > +        bars[index].type = HVM_PT_BAR_MEM32;
> > +
> > +    if ( bars[index].type == HVM_PT_BAR_MEM32 ||
> > +         bars[index].type == HVM_PT_BAR_MEM64_LO )
> > +    {
> > +        /* Size the BAR and map it. */
> > +        rc = pci_size_bar(seg, bus, slot, func, real_offset - 
> > handler->offset,
> > +                          num_bars, &index, &addr, &size);
> > +        if ( rc )
> > +        {
> > +            printk_pdev(s->pdev, XENLOG_ERR, "unable to size BAR#%d\n",
> > +                        index);
> > +            return rc;
> > +        }
> > +
> > +        if ( size == 0 )
> > +            bars[index].type = HVM_PT_BAR_UNUSED;
> > +        else
> > +        {
> > +            printk_pdev(s->pdev, XENLOG_DEBUG,
> > +                        "Mapping BAR#%u: %#lx size: %u\n", index, addr, 
> > size);
> > +            rc = modify_mmio_11(s->pdev->domain, PFN_DOWN(addr),
> > +                                DIV_ROUND_UP(size, PAGE_SIZE), true);
> > +            if ( rc )
> > +            {
> > +                printk_pdev(s->pdev, XENLOG_ERR,
> > +                            "failed to map BAR#%d into memory map: %d\n",
> > +                            index, rc);
> > +                return rc;
> > +            }
> > +        }
> > +    }
> > +
> > +    *data = bars[index].type == HVM_PT_BAR_UNUSED ?
> > HVM_PT_INVALID_REG : val;
> > +    return 0;
> > +}
> > +
> > +/* Only allow writes to check the size of the BARs */ static int
> > +allow_bar_write(struct hvm_pt_bar *bar, struct hvm_pt_reg *reg,
> > +                           struct pci_dev *pdev, uint32_t val) {
> > +    uint32_t mask;
> > +
> > +    if ( bar->type == HVM_PT_BAR_MEM64_HI )
> > +        mask = ~0;
> > +    else
> > +        mask = (uint32_t) PCI_BASE_ADDRESS_MEM_MASK;
> > +
> > +    if ( val != ~0 && (val & mask) != (reg->val.dword & mask) )
> > +    {
> > +        printk_pdev(pdev, XENLOG_ERR,
> > +                "changing the position of the BARs is not yet supported: 
> > %#x\n",
> > +                    val);
> 
> This doesn't seem to quite tally with commit comment. Can BARs be 
> re-programmed or not?

Right, I got messed up in the commit meesage. This _prevents_ remapping BARs 
(which would also not work anyway). Further code to allow remapping BARs can 
be based on top of this handlers.

Roger.

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