* [RFC PATCH 7/8]: PVH: grant changes
@ 2012-08-16 1:06 Mukesh Rathor
2012-08-17 9:52 ` Ian Campbell
0 siblings, 1 reply; 4+ messages in thread
From: Mukesh Rathor @ 2012-08-16 1:06 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Xen-devel@lists.xensource.com
---
drivers/xen/gntdev.c | 2 +-
drivers/xen/grant-table.c | 26 +++++++++++++++++++++-----
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 1ffd03b..ad2d28e 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -802,7 +802,7 @@ static int __init gntdev_init(void)
if (!xen_domain())
return -ENODEV;
- use_ptemod = xen_pv_domain();
+ use_ptemod = xen_pv_domain() && !xen_pvh_domain();
err = misc_register(&gntdev_miscdev);
if (err != 0) {
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 0bfc1ef..2430133 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -974,14 +974,19 @@ static void gnttab_unmap_frames_v2(void)
static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
{
struct gnttab_setup_table setup;
- unsigned long *frames;
+ unsigned long *frames, start_gpfn;
unsigned int nr_gframes = end_idx + 1;
int rc;
- if (xen_hvm_domain()) {
+ if (xen_hvm_domain() || xen_pvh_domain()) {
struct xen_add_to_physmap xatp;
unsigned int i = end_idx;
rc = 0;
+
+ if (xen_hvm_domain())
+ start_gpfn = xen_hvm_resume_frames >> PAGE_SHIFT;
+ else
+ start_gpfn = virt_to_pfn(gnttab_shared.addr);
/*
* Loop backwards, so that the first hypercall has the largest
* index, ensuring that the table will grow only once.
@@ -990,7 +995,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
xatp.domid = DOMID_SELF;
xatp.idx = i;
xatp.space = XENMAPSPACE_grant_table;
- xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
+ xatp.gpfn = start_gpfn + i;
rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
if (rc != 0) {
printk(KERN_WARNING
@@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
int rc;
struct gnttab_set_version gsv;
- if (xen_hvm_domain())
+ if (xen_hvm_domain() || xen_pvh_domain())
gsv.version = 1;
else
gsv.version = 2;
@@ -1081,13 +1086,24 @@ static void gnttab_request_version(void)
int gnttab_resume(void)
{
unsigned int max_nr_gframes;
+ char *kmsg="Failed to kmalloc pages for pv in hvm grant frames\n";
gnttab_request_version();
max_nr_gframes = gnttab_max_grant_frames();
if (max_nr_gframes < nr_grant_frames)
return -ENOSYS;
- if (xen_pv_domain())
+ /* PVH note: xen will free existing kmalloc'd mfn in
+ * XENMEM_add_to_physmap */
+ if (xen_pvh_domain() && !gnttab_shared.addr) {
+ gnttab_shared.addr =
+ kmalloc(max_nr_gframes * PAGE_SIZE, GFP_KERNEL);
+ if ( !gnttab_shared.addr ) {
+ printk(KERN_WARNING "%s", kmsg);
+ return -ENOMEM;
+ }
+ }
+ if (xen_pv_domain() || xen_pvh_domain())
return gnttab_map(0, nr_grant_frames - 1);
if (gnttab_shared.addr == NULL) {
--
1.7.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [RFC PATCH 7/8]: PVH: grant changes
2012-08-16 1:06 [RFC PATCH 7/8]: PVH: grant changes Mukesh Rathor
@ 2012-08-17 9:52 ` Ian Campbell
2012-08-18 0:06 ` Mukesh Rathor
0 siblings, 1 reply; 4+ messages in thread
From: Ian Campbell @ 2012-08-17 9:52 UTC (permalink / raw)
To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk
On Thu, 2012-08-16 at 02:06 +0100, Mukesh Rathor wrote:
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 0bfc1ef..2430133 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -974,14 +974,19 @@ static void gnttab_unmap_frames_v2(void)
> static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
> {
> struct gnttab_setup_table setup;
> - unsigned long *frames;
> + unsigned long *frames, start_gpfn;
> unsigned int nr_gframes = end_idx + 1;
> int rc;
>
> - if (xen_hvm_domain()) {
> + if (xen_hvm_domain() || xen_pvh_domain()) {
> struct xen_add_to_physmap xatp;
> unsigned int i = end_idx;
> rc = 0;
> +
> + if (xen_hvm_domain())
> + start_gpfn = xen_hvm_resume_frames >> PAGE_SHIFT;
> + else
> + start_gpfn = virt_to_pfn(gnttab_shared.addr);
I wonder why the HVM case doesn't already use
virt_to_pfn(gnttab_shared.addr) since it appears to set
gnttab_shared.addr:
gnttab_shared.addr = ioremap(xen_hvm_resume_frames,
PAGE_SIZE * max_nr_gframes);
Perhaps the result of ioremap isn't amenable to virt_to_pfn (I can never
remember off hand)
> /*
> * Loop backwards, so that the first hypercall has the largest
> * index, ensuring that the table will grow only once.
> @@ -990,7 +995,7 @@ static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
> xatp.domid = DOMID_SELF;
> xatp.idx = i;
> xatp.space = XENMAPSPACE_grant_table;
> - xatp.gpfn = (xen_hvm_resume_frames >> PAGE_SHIFT) + i;
> + xatp.gpfn = start_gpfn + i;
> rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
> if (rc != 0) {
> printk(KERN_WARNING
> @@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
> int rc;
> struct gnttab_set_version gsv;
>
> - if (xen_hvm_domain())
> + if (xen_hvm_domain() || xen_pvh_domain())
Does something stop pvh using v2?
Is it a hypervisor implementation thing? In which case it might be
better for GNTTABOP_set_version to explicitly fail the set attempt? (the
same may well apply to HVM for all I know...)
> gsv.version = 1;
> else
> gsv.version = 2;
> @@ -1081,13 +1086,24 @@ static void gnttab_request_version(void)
> int gnttab_resume(void)
> {
> unsigned int max_nr_gframes;
> + char *kmsg="Failed to kmalloc pages for pv in hvm grant frames\n";
>
> gnttab_request_version();
> max_nr_gframes = gnttab_max_grant_frames();
> if (max_nr_gframes < nr_grant_frames)
> return -ENOSYS;
>
> - if (xen_pv_domain())
> + /* PVH note: xen will free existing kmalloc'd mfn in
> + * XENMEM_add_to_physmap */
> + if (xen_pvh_domain() && !gnttab_shared.addr) {
> + gnttab_shared.addr =
> + kmalloc(max_nr_gframes * PAGE_SIZE, GFP_KERNEL);
> + if ( !gnttab_shared.addr ) {
> + printk(KERN_WARNING "%s", kmsg);
Why this construct instead of just the string literal?
> + return -ENOMEM;
> + }
> + }
> + if (xen_pv_domain() || xen_pvh_domain())
> return gnttab_map(0, nr_grant_frames - 1);
>
> if (gnttab_shared.addr == NULL) {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC PATCH 7/8]: PVH: grant changes
2012-08-17 9:52 ` Ian Campbell
@ 2012-08-18 0:06 ` Mukesh Rathor
2012-08-18 8:22 ` Ian Campbell
0 siblings, 1 reply; 4+ messages in thread
From: Mukesh Rathor @ 2012-08-18 0:06 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk
On Fri, 17 Aug 2012 10:52:30 +0100
Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-08-16 at 02:06 +0100, Mukesh Rathor wrote:
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 0bfc1ef..2430133 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp); if (rc != 0) {
> > printk(KERN_WARNING
> > @@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
> > int rc;
> > struct gnttab_set_version gsv;
> >
> > - if (xen_hvm_domain())
> > + if (xen_hvm_domain() || xen_pvh_domain())
>
> Does something stop pvh using v2?
I had some issue related to grstatus field that was added, so punted it
for now. It's phase II which is a big phase now :) :)...
> >
> > - if (xen_pv_domain())
> > + /* PVH note: xen will free existing kmalloc'd mfn in
> > + * XENMEM_add_to_physmap */
> > + if (xen_pvh_domain() && !gnttab_shared.addr) {
> > + gnttab_shared.addr =
> > + kmalloc(max_nr_gframes * PAGE_SIZE,
> > GFP_KERNEL);
> > + if ( !gnttab_shared.addr ) {
> > + printk(KERN_WARNING "%s", kmsg);
>
> Why this construct instead of just the string literal?
To keep line overflow. I dont' like code spanning 80 columns. If you split,
then you can't grep.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC PATCH 7/8]: PVH: grant changes
2012-08-18 0:06 ` Mukesh Rathor
@ 2012-08-18 8:22 ` Ian Campbell
0 siblings, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2012-08-18 8:22 UTC (permalink / raw)
To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com, Konrad Rzeszutek Wilk
> > > HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp); if (rc != 0) {
> > > printk(KERN_WARNING
> > > @@ -1053,7 +1058,7 @@ static void gnttab_request_version(void)
> > > int rc;
> > > struct gnttab_set_version gsv;
> > >
> > > - if (xen_hvm_domain())
> > > + if (xen_hvm_domain() || xen_pvh_domain())
> >
> > Does something stop pvh using v2?
>
> I had some issue related to grstatus field that was added, so punted it
> for now. It's phase II which is a big phase now :) :)...
It seems like it's got a lot of most independent bits in it, so
hopefully you should get some help ;-)
> > >
> > > - if (xen_pv_domain())
> > > + /* PVH note: xen will free existing kmalloc'd mfn in
> > > + * XENMEM_add_to_physmap */
> > > + if (xen_pvh_domain() && !gnttab_shared.addr) {
> > > + gnttab_shared.addr =
> > > + kmalloc(max_nr_gframes * PAGE_SIZE,
> > > GFP_KERNEL);
> > > + if ( !gnttab_shared.addr ) {
> > > + printk(KERN_WARNING "%s", kmsg);
> >
> > Why this construct instead of just the string literal?
>
> To keep line overflow. I dont' like code spanning 80 columns. If you split,
> then you can't grep.
FWIW CodingStyle relaxes the 80 column limit for literal strings for
exactly this reason.
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-18 8:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-16 1:06 [RFC PATCH 7/8]: PVH: grant changes Mukesh Rathor
2012-08-17 9:52 ` Ian Campbell
2012-08-18 0:06 ` Mukesh Rathor
2012-08-18 8:22 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).