RE: [Xen-ia64-devel] [PATCH][RFC][TAKE4] the P2M/VP patches

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>From: Isaku Yamahata
>Sent: 2006年4月7日 12:16
>
>* summary
>With these patches, dom0, domU boot and vnif, balloon works.

Great progress.

>9504:c50a58f28451_fix_grant_entry_t_frame.patch
>        change the type of grant_entry::frame from uint32_t to
>unsigned long.
>        In theory IA64 supports 50 bits physical address,
>        36 bits is needed with 16KBytes page size.
>        As Kevin pointed out xen/ia64 can work fine for several years
>        without this patch.
>        If this patch is rejected, we can go without this patch.

At least one reason to accept this patch is that current grant_table.h
itself is even using both. Mfn of gnttab_transfer is defined as unsigned 
long, however frames of other structures for uint32_t. You may push 
as a cleanup reason.

>
>9519:d6c77b041a70_page_to_bus.patch
>        introduce page_to_bus() and used for pci-dma-xen.c and
>swiotlb.c
>        On xen/ia64 with the P2M/VP model pseudo physical address
>is fully
>        virtualized so it defines as
>        xen_features(XENFEAT_auto_translated_physmap) = 1.
>        In this case page_to_phys(sg[i].page) should return pseudo
>physical
>        address like pfn_to_mfn() and its families.
>        However dma is not virtualized, it can't be used for
>pci-dma-xen.c,
>        swiotlb.c.
>
>        If this patch is rejected, we can introduce xenLinux/ia64-specific
>        pci-dma-xen.c swiotlb.c. But such a divergence is not desirable.

Seems reasonable. Just one small comment, since xen/ia64 currently
depends on auto_translated mode, is it possible to make your solution
common, instead of only special case for xen/ia64? By your patch, x86
still can't take auto_translated mode as a backend. Not sure whether
easy/clean to make it, maybe a submode of auto_translated?

>9526:f93f01f21f37_arch_specific_xen_feature.patch
>        allow arch to define arch-specific xen_feature()
>        Xen/ia64 with the P2M/VP model define
>        xen_features(XENFEAT_auto_translated_physmap) = 1.
>        It is NOT run-time feature. It is desirable to code it explicitly
>        like
>        static inline int
>        xen_feature(int flag)
>        {
>               switch (flag)
>               {
>               case XENFEAT_auto_translated_physmap:
>                       return 1;
>               default:
>                       return xen_features[flag];
>               }
>               /* NOTREACHED */
>        }
>        This patch allows it.

Does this patch only save one hypercall overhead? We can always 
tell guest the auto_translated bit is true when guest hypercalls to 
query feature bits into xen_feature array.

>
>
>* proposal for the merge of the P2M/VP patches
>It was decided that the P2M/VP patches would go to
>xen-ia64-unstable-Intel.hg at first and then would be pulled up to
>xen-ia64-unstable.hg once it was stabilized.
>However the P2M/VP patches have become too big.
>(We didn't expect at that time, did we?)
>It might be very difficult to merge at a time, I think.
>So I propose merging gradually reducing the patch size.
>The P2M/VP patches have a compile-time option.
>The option is disabled by default at first.

Yes, I think it's good for your patches to be in xen-ia64-unstable 
gradually with compile option there. This is a typical way how a 
complete feature is added into mainstream, like recent supervisor 
mode for example. Yes, once after getting community consensus. :-)

Thanks,
Kevin

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel