Isaku Yamahata wrote:
> On Thu, Oct 16, 2008 at 07:37:51PM +0800, Xu, Anthony wrote:
>> use VTD to assing device, guest port may not be equal to host port.
>> Change ioports_permit_access interface
>> add deassign_domain_mmio_page interface
>>
>> Signed-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >
>
> Some comments.
>
> - ioports_permit_access()
> x86 didn't change its prototype.
> Why does only ia64 need to change it?
> Or will x86 also change it?
In x86 side, only it is identity mapping, guest can access the port directly.
If it is not identity mapping, guest can not access the port directly, at this
situation,
xen will intercept io port access first, and xen access the corresponding
physical port.
In ia64 side, guest can acess all assigned port whenever it is identity mapping
or not.
So we need to change the interface.
>
> I suppose, it would be nice to map the port io area
> to arbitrary guest physical area.
> But I'm not sure how x86 will go with XEN_DOMCTL_ioport_mapping.
>
> - deassign_domain_mmio_page()
> calling __assgin_domain_page() may result in page reference count
> leak because the corresponding p2m entry may be changed to another
> value.
> So you want to modify zap_domain_page_one() so that it accepts
> iomem page and call it from deassign_domain_mmio_page().
There is no page_info for mmio page, I didn't see reference count issue.
If VTD is enabled, in the life of guest, p2m can not be changed,
Because it VTD operation hit a page table miss, the DMA operation can not be
resumed.
Thanks,
Anthony
>
> - probably it would better to split this patch into two ones.
>
> thanks,
>
>> use VTD to assing device, guest port may not be equal to host port.
>> Change ioports_permit_access interface
>> add deassign_domain_mmio_page interface
>>
>> Signed-off-by; Anthony Xu < anthony.xu@xxxxxxxxx >
>>
>>
>>
>>
>>
>> diff -r 42c7733c1a2a xen/arch/ia64/xen/dom0_ops.c
>> --- a/xen/arch/ia64/xen/dom0_ops.c Thu Oct 16 18:18:39 2008 +0800
>> +++ b/xen/arch/ia64/xen/dom0_ops.c Thu Oct 16 19:13:01 2008 +0800
>> @@ -203,7 +203,7 @@ ret = 0;
>> else {
>> if (op->u.ioport_permission.allow_access)
>> - ret = ioports_permit_access(d, fp, lp);
>> + ret = ioports_permit_access(d, fp, fp, lp);
>> else ret = ioports_deny_access(d, fp, lp);
>> } @@ -522,7 +522,7 @@
>> fp = space_number << IO_SPACE_BITS;
>> lp = fp | 0xffff;
>>
>> - return ioports_permit_access(d, fp, lp);
>> + return ioports_permit_access(d, fp, fp, lp);
>> }
>>
>> unsigned long
>> diff -r 42c7733c1a2a xen/arch/ia64/xen/domain.c
>> --- a/xen/arch/ia64/xen/domain.c Thu Oct 16 18:18:39 2008 +0800
>> +++ b/xen/arch/ia64/xen/domain.c Thu Oct 16 19:13:01 2008 +0800
>> @@ -1995,7 +1995,7 @@ BUG();
>> if (irqs_permit_access(d, 0, NR_IRQS-1))
>> BUG();
>> - if (ioports_permit_access(d, 0, 0xffff))
>> + if (ioports_permit_access(d, 0, 0, 0xffff))
>> BUG();
>> }
>>
>> diff -r 42c7733c1a2a xen/arch/ia64/xen/mm.c
>> --- a/xen/arch/ia64/xen/mm.c Thu Oct 16 18:18:39 2008 +0800
>> +++ b/xen/arch/ia64/xen/mm.c Thu Oct 16 19:13:01 2008 +0800 @@
>> -984,15 +984,22 @@ ASSIGN_writable |
>> ASSIGN_pgc_allocated); }
>>
>> +/*
>> + * Inpurt
>> + * fgp: first guest port
>> + * fmp: first machine port
>> + * lmp: last machine port
>> + */
>> int
>> -ioports_permit_access(struct domain *d, unsigned int fp, unsigned
>> int lp) +ioports_permit_access(struct domain *d, unsigned int fgp,
>> + unsigned int fmp, unsigned int lmp)
>> {
>> struct io_space *space;
>> - unsigned long mmio_start, mmio_end, mach_start;
>> + unsigned long mmio_start, mach_start, mach_end; int ret;
>>
>> - if (IO_SPACE_NR(fp) >= num_io_spaces) {
>> - dprintk(XENLOG_WARNING, "Unknown I/O Port range 0x%x -
>> 0x%x\n", fp, lp); + if (IO_SPACE_NR(fmp) >= num_io_spaces) {
>> + dprintk(XENLOG_WARNING, "Unknown I/O Port range 0x%x -
>> 0x%x\n", fmp, lmp); return -EFAULT; }
>>
>> @@ -1006,42 +1013,44 @@
>> * I/O port spaces and thus will number port spaces differently.
>> * This is ok, they don't make use of this interface. */
>> - ret = rangeset_add_range(d->arch.ioport_caps, fp, lp);
>> + ret = rangeset_add_range(d->arch.ioport_caps, fmp, lmp);
>> if (ret != 0) return ret;
>>
>> - space = &io_space[IO_SPACE_NR(fp)];
>> + space = &io_space[IO_SPACE_NR(fmp)];
>>
>> /* Legacy I/O on dom0 is already setup */
>> if (d == dom0 && space == &io_space[0])
>> return 0;
>>
>> - fp = IO_SPACE_PORT(fp);
>> - lp = IO_SPACE_PORT(lp);
>> + fmp = IO_SPACE_PORT(fmp);
>> + lmp = IO_SPACE_PORT(lmp);
>>
>> if (space->sparse) {
>> - mmio_start = IO_SPACE_SPARSE_ENCODING(fp) & PAGE_MASK;
>> - mmio_end = PAGE_ALIGN(IO_SPACE_SPARSE_ENCODING(lp));
>> + mach_start = IO_SPACE_SPARSE_ENCODING(fmp) & PAGE_MASK;
>> + mach_end = PAGE_ALIGN(IO_SPACE_SPARSE_ENCODING(lmp));
>> } else {
>> - mmio_start = fp & PAGE_MASK;
>> - mmio_end = PAGE_ALIGN(lp);
>> + mach_start = fmp & PAGE_MASK;
>> + mach_end = PAGE_ALIGN(lmp);
>> }
>>
>> /*
>> * The "machine first port" is not necessarily identity mapped
>> * to the guest first port. At least for the legacy range.
>> */ - mach_start = mmio_start | __pa(space->mmio_base);
>> + mach_start = mach_start | __pa(space->mmio_base);
>> + mach_end = mach_end | __pa(space->mmio_base);
>>
>> - if (space == &io_space[0]) {
>> + mmio_start = IO_SPACE_SPARSE_ENCODING(fgp) & PAGE_MASK; +
>> + if ( VMX_DOMAIN(d->vcpu[0]))
>> + mmio_start |= LEGACY_IO_START;
>> + else if (space == &io_space[0])
>> mmio_start |= IO_PORTS_PADDR;
>> - mmio_end |= IO_PORTS_PADDR;
>> - } else {
>> + else
>> mmio_start |= __pa(space->mmio_base);
>> - mmio_end |= __pa(space->mmio_base);
>> - }
>>
>> - while (mmio_start <= mmio_end) {
>> + while (mach_start < mach_end) {
>> __assign_domain_page(d, mmio_start,
>> mach_start, ASSIGN_nocache | ASSIGN_direct_io);
>> mmio_start += PAGE_SIZE;
>> @@ -1091,7 +1100,9 @@
>> mmio_end = PAGE_ALIGN(lp_base);
>> }
>>
>> - if (space == &io_space[0] && d != dom0)
>> + if (VMX_DOMAIN(d->vcpu[0]))
>> + mmio_base = LEGACY_IO_START;
>> + else if (space == &io_space[0] && d != dom0)
>> mmio_base = IO_PORTS_PADDR;
>> else
>> mmio_base = __pa(space->mmio_base);
>> @@ -1217,6 +1228,33 @@
>>
>> return mpaddr;
>> }
>> +
>> +int
>> +deassign_domain_mmio_page(struct domain *d, unsigned long mpaddr,
>> + unsigned long phys_addr, unsigned long size ) +{
>> + unsigned long addr = mpaddr & PAGE_MASK;
>> + unsigned long end = PAGE_ALIGN(mpaddr + size); +
>> + if (size == 0) {
>> + gdprintk(XENLOG_INFO, "%s: domain %p mpaddr 0x%lx size =
>> 0x%lx\n", + __func__, d, mpaddr, size);
>> + }
>> + if (!efi_mmio(phys_addr, size)) {
>> +#ifndef NDEBUG
>> + gdprintk(XENLOG_INFO, "%s: domain %p mpaddr 0x%lx size =
>> 0x%lx\n", + __func__, d, mpaddr, size);
>> +#endif
>> + return -EINVAL;
>> + }
>> +
>> + for (; addr < end; addr += PAGE_SIZE ) {
>> + __assign_domain_page(d, addr, 0,
>> + ASSIGN_nocache | ASSIGN_direct_io); + }
>> + return 0;
>> +}
>> +
>>
>> unsigned long
>> assign_domain_mach_page(struct domain *d,
>> diff -r 42c7733c1a2a xen/include/asm-ia64/iocap.h
>> --- a/xen/include/asm-ia64/iocap.h Thu Oct 16 18:18:39 2008 +0800
>> +++ b/xen/include/asm-ia64/iocap.h Thu Oct 16 19:13:01 2008 +0800
>> @@ -7,7 +7,7 @@ #ifndef __IA64_IOCAP_H__
>> #define __IA64_IOCAP_H__
>>
>> -extern int ioports_permit_access(struct domain *d,
>> +extern int ioports_permit_access(struct domain *d, unsigned int gs,
>> unsigned int s, unsigned int e);
>> extern int ioports_deny_access(struct domain *d,
>> unsigned int s, unsigned int e);
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home