| Message ID | 1508258793-5690-5-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [Xen-devel] xenconsole: Define and use a macro INVALID_XEN_PFN instead of -1 | expand |
>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote: > --- a/tools/console/daemon/io.c > +++ b/tools/console/daemon/io.c > @@ -658,12 +658,12 @@ static void console_unmap_interface(struct console *con) > { > if (con->interface == NULL) > return; > - if (xgt_handle && con->ring_ref == -1) > + if (xgt_handle && con->ring_ref == INVALID_XEN_PFN) > xengnttab_unmap(xgt_handle, con->interface, 1); > else > munmap(con->interface, XC_PAGE_SIZE); > con->interface = NULL; > - con->ring_ref = -1; > + con->ring_ref = INVALID_XEN_PFN; > } > > static int console_create_ring(struct console *con) > @@ -698,7 +698,7 @@ static int console_create_ring(struct console *con) > free(type); > > /* If using ring_ref and it has changed, remap */ > - if (ring_ref != con->ring_ref && con->ring_ref != -1) > + if (ring_ref != con->ring_ref && con->ring_ref != INVALID_XEN_PFN) > console_unmap_interface(con); > > if (!con->interface && xgt_handle && con->use_gnttab) { > @@ -706,7 +706,7 @@ static int console_create_ring(struct console *con) > con->interface = xengnttab_map_grant_ref(xgt_handle, > dom->domid, GNTTAB_RESERVED_CONSOLE, > PROT_READ|PROT_WRITE); > - con->ring_ref = -1; > + con->ring_ref = INVALID_XEN_PFN; > } > if (!con->interface) { > /* Fall back to xc_map_foreign_range */ > @@ -812,7 +812,7 @@ static int console_init(struct console *con, struct domain *dom, void **data) > con->master_pollfd_idx = -1; > con->slave_fd = -1; > con->log_fd = -1; > - con->ring_ref = -1; > + con->ring_ref = INVALID_XEN_PFN; > con->local_port = -1; > con->remote_port = -1; > con->xce_pollfd_idx = -1; > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -37,6 +37,8 @@ > #error "Unsupported architecture" > #endif > > +#define INVALID_XEN_PFN (~(xen_pfn_t)0) As said before, the uses of this which you introduce don't warrant this addition to the public interface (which, if it was added, also should start with XEN_). I'm not going to NAK such a (corrected) addition to the public interface, but given the users I'm also not going to ACK it (but perhaps another REST maintainer would). Jan
On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote: > >>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote: > > --- a/tools/console/daemon/io.c > > +++ b/tools/console/daemon/io.c > > @@ -658,12 +658,12 @@ static void console_unmap_interface(struct console *con) > > { > > if (con->interface == NULL) > > return; > > - if (xgt_handle && con->ring_ref == -1) > > + if (xgt_handle && con->ring_ref == INVALID_XEN_PFN) > > xengnttab_unmap(xgt_handle, con->interface, 1); > > else > > munmap(con->interface, XC_PAGE_SIZE); > > con->interface = NULL; > > - con->ring_ref = -1; > > + con->ring_ref = INVALID_XEN_PFN; > > } > > > > static int console_create_ring(struct console *con) > > @@ -698,7 +698,7 @@ static int console_create_ring(struct console *con) > > free(type); > > > > /* If using ring_ref and it has changed, remap */ > > - if (ring_ref != con->ring_ref && con->ring_ref != -1) > > + if (ring_ref != con->ring_ref && con->ring_ref != INVALID_XEN_PFN) > > console_unmap_interface(con); > > > > if (!con->interface && xgt_handle && con->use_gnttab) { > > @@ -706,7 +706,7 @@ static int console_create_ring(struct console *con) > > con->interface = xengnttab_map_grant_ref(xgt_handle, > > dom->domid, GNTTAB_RESERVED_CONSOLE, > > PROT_READ|PROT_WRITE); > > - con->ring_ref = -1; > > + con->ring_ref = INVALID_XEN_PFN; > > } > > if (!con->interface) { > > /* Fall back to xc_map_foreign_range */ > > @@ -812,7 +812,7 @@ static int console_init(struct console *con, struct domain *dom, void **data) > > con->master_pollfd_idx = -1; > > con->slave_fd = -1; > > con->log_fd = -1; > > - con->ring_ref = -1; > > + con->ring_ref = INVALID_XEN_PFN; > > con->local_port = -1; > > con->remote_port = -1; > > con->xce_pollfd_idx = -1; > > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -37,6 +37,8 @@ > > #error "Unsupported architecture" > > #endif > > > > +#define INVALID_XEN_PFN (~(xen_pfn_t)0) > > As said before, the uses of this which you introduce don't warrant > this addition to the public interface (which, if it was added, also > should start with XEN_). I'm not going to NAK such a (corrected) > addition to the public interface, but given the users I'm also not > going to ACK it (but perhaps another REST maintainer would). > I agree with you here. We don't need this in public interface yet.
Hi Wei, On 10/18/2017 12:53 PM, Wei Liu wrote: > On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote: >>>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote: >>> --- a/xen/include/public/xen.h >>> +++ b/xen/include/public/xen.h >>> @@ -37,6 +37,8 @@ >>> #error "Unsupported architecture" >>> #endif >>> >>> +#define INVALID_XEN_PFN (~(xen_pfn_t)0) >> >> As said before, the uses of this which you introduce don't warrant >> this addition to the public interface (which, if it was added, also >> should start with XEN_). I'm not going to NAK such a (corrected) >> addition to the public interface, but given the users I'm also not >> going to ACK it (but perhaps another REST maintainer would). >> > > I agree with you here. We don't need this in public interface yet. Couldn't this new define be used in place like xc_mem_access.c (they have a plain ~0UL for invalid GFN) or even LIBXL_INVALID_GFN? Cheers,
>>> On 18.10.17 at 15:05, <julien.grall@linaro.org> wrote: > On 10/18/2017 12:53 PM, Wei Liu wrote: >> On Wed, Oct 18, 2017 at 04:02:45AM -0600, Jan Beulich wrote: >>>>>> On 17.10.17 at 18:46, <bhupinder.thakur@linaro.org> wrote: >>>> --- a/xen/include/public/xen.h >>>> +++ b/xen/include/public/xen.h >>>> @@ -37,6 +37,8 @@ >>>> #error "Unsupported architecture" >>>> #endif >>>> >>>> +#define INVALID_XEN_PFN (~(xen_pfn_t)0) >>> >>> As said before, the uses of this which you introduce don't warrant >>> this addition to the public interface (which, if it was added, also >>> should start with XEN_). I'm not going to NAK such a (corrected) >>> addition to the public interface, but given the users I'm also not >>> going to ACK it (but perhaps another REST maintainer would). >>> >> >> I agree with you here. We don't need this in public interface yet. > > Couldn't this new define be used in place like xc_mem_access.c (they > have a plain ~0UL for invalid GFN) or even LIBXL_INVALID_GFN? Sure it could be, but it should be introduced to the public header when a hypercall producer and/or consumer appears, not for the internal purposes of xenconsole. Jan
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c index 1839973..9129f5a 100644 --- a/tools/console/daemon/io.c +++ b/tools/console/daemon/io.c @@ -658,12 +658,12 @@ static void console_unmap_interface(struct console *con) { if (con->interface == NULL) return; - if (xgt_handle && con->ring_ref == -1) + if (xgt_handle && con->ring_ref == INVALID_XEN_PFN) xengnttab_unmap(xgt_handle, con->interface, 1); else munmap(con->interface, XC_PAGE_SIZE); con->interface = NULL; - con->ring_ref = -1; + con->ring_ref = INVALID_XEN_PFN; } static int console_create_ring(struct console *con) @@ -698,7 +698,7 @@ static int console_create_ring(struct console *con) free(type); /* If using ring_ref and it has changed, remap */ - if (ring_ref != con->ring_ref && con->ring_ref != -1) + if (ring_ref != con->ring_ref && con->ring_ref != INVALID_XEN_PFN) console_unmap_interface(con); if (!con->interface && xgt_handle && con->use_gnttab) { @@ -706,7 +706,7 @@ static int console_create_ring(struct console *con) con->interface = xengnttab_map_grant_ref(xgt_handle, dom->domid, GNTTAB_RESERVED_CONSOLE, PROT_READ|PROT_WRITE); - con->ring_ref = -1; + con->ring_ref = INVALID_XEN_PFN; } if (!con->interface) { /* Fall back to xc_map_foreign_range */ @@ -812,7 +812,7 @@ static int console_init(struct console *con, struct domain *dom, void **data) con->master_pollfd_idx = -1; con->slave_fd = -1; con->log_fd = -1; - con->ring_ref = -1; + con->ring_ref = INVALID_XEN_PFN; con->local_port = -1; con->remote_port = -1; con->xce_pollfd_idx = -1; diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 308109f..fc383ca 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -37,6 +37,8 @@ #error "Unsupported architecture" #endif +#define INVALID_XEN_PFN (~(xen_pfn_t)0) + #ifndef __ASSEMBLY__ /* Guest handles for primitive C types. */ DEFINE_XEN_GUEST_HANDLE(char);
xenconsole will use a new macro INVALID_XEN_PFN instead of -1 for initializing ring-ref. Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- CC: Ian Jackson <ian.jackson@eu.citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: George Dunlap <George.Dunlap@eu.citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> tools/console/daemon/io.c | 10 +++++----- xen/include/public/xen.h | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-)