Changeset View
Standalone View
sys/xen/info.c
- This file was added.
/*- | |||||
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD | |||||
* | |||||
* Copyright (c) 2018 Citrix Systems, Inc. | |||||
* All rights reserved. | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
*/ | |||||
#include <sys/cdefs.h> | |||||
__FBSDID("$FreeBSD$"); | |||||
#include <sys/types.h> | |||||
#include <sys/param.h> /* required by xen/xen-os.h */ | |||||
#include <machine/atomic.h> /* required by xen/xen-os.h */ | |||||
#include <xen/xen-os.h> | |||||
#include <xen/hvm.h> | |||||
#include <xen/interface/event_channel.h> | |||||
#include <xen/interface/hvm/params.h> | |||||
/*-------------------------------- Global Data -------------------------------*/ | |||||
/** | |||||
* Start info flags. ATM this only used to store the initial domain flag for | |||||
* PVHv2, and it's always empty for HVM guests. | |||||
*/ | |||||
uint32_t hvm_start_flags; | |||||
/*---------------------- XEN Hypervisor Probe and Setup ----------------------*/ | |||||
/* HVM/PVH start_info accessors */ | |||||
static vm_paddr_t | |||||
hvm_get_xenstore_mfn(void) | |||||
{ | |||||
return (hvm_get_parameter(HVM_PARAM_STORE_PFN)); | |||||
} | |||||
static evtchn_port_t | |||||
hvm_get_xenstore_evtchn(void) | |||||
{ | |||||
return (hvm_get_parameter(HVM_PARAM_STORE_EVTCHN)); | |||||
} | |||||
static vm_paddr_t | |||||
hvm_get_console_mfn(void) | |||||
{ | |||||
return (hvm_get_parameter(HVM_PARAM_CONSOLE_PFN)); | |||||
} | |||||
static evtchn_port_t | |||||
hvm_get_console_evtchn(void) | |||||
{ | |||||
return (hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN)); | |||||
} | |||||
static uint32_t | |||||
hvm_get_start_flags(void) | |||||
{ | |||||
return (hvm_start_flags); | |||||
} | |||||
#if !defined(__x86_64__) && !defined(__x86__) | |||||
const | |||||
mhorne: Why is the `const` required, or useful, at all? | |||||
ehem_freebsd_m5p.comAuthorUnsubmitted Done Inline ActionsSince on ARM (and likely RISC-V) it is constant. Compiler gets to do more optimizations around access to the data. If at some future point a bug caused memory corruption, this structure would be safe; if a future security vulnerability evolved which involved this data, making it constant could mitigate it. Mostly it is constant, so mark it so. ehem_freebsd_m5p.com: Since on ARM (and likely RISC-V) it **is** constant. Compiler gets to do more optimizations… | |||||
mhorneUnsubmitted Not Done Inline ActionsSure, I take your point that optimization could save the indirection, however, a smart compiler could deduce this regardless of the const qualifier. In my opinion, it is best to limit ifdef checks for CPU architecture in machine-independent code whenever it is non-trivial to do so. In either case, my main feeling is that it does not make sense to make specific provisions for !x86 support until that support is finalized/reviewable somewhere. It can always be added in a later commit. Otherwise, this patch seems like a useful first step towards the arm64 support. mhorne: Sure, I take your point that optimization could save the indirection, however, a smart compiler… | |||||
ehem_freebsd_m5p.comAuthorUnsubmitted Done Inline Actions
Can also be valuable during development. If I've got a data structure which I need to preserve, if I look up a function declaration and some incoming data is marked const I know I can pass it in and not need to keep a duplicate copy. I hold the view far too little is declared const.
It is out there, but isn't ready for review. I'm waiting for an official okay on FreeBSD's license. Alas Xen is in release mode, so the original author is busy; I believe the author was okay with FreeBSD's license, just I want formal approval. So far a FreeBSD aarch64 DomU has been observed starting its boot process (guess how D28988 was observed in real life). vCPU and memory probing work, but at some point near/during xenpv0 probing things go crunch. You want to trying a build? ehem_freebsd_m5p.com: > Sure, I take your point that optimization could save the indirection, however, a smart… | |||||
#endif | |||||
roygerUnsubmitted Not Done Inline ActionsI would be fine if you dropped the legacy_info (and related helpers), and instead made the helpers in xen-os.h just use hvm_get_parameter (like the default hypervisor_info implementation). Then we could drop the indirection. There's no reason to carry the legacy stuff any longer. royger: I would be fine if you dropped the legacy_info (and related helpers), and instead made the… | |||||
struct hypervisor_info hypervisor_info = { | |||||
.get_xenstore_mfn = hvm_get_xenstore_mfn, | |||||
.get_xenstore_evtchn = hvm_get_xenstore_evtchn, | |||||
.get_console_mfn = hvm_get_console_mfn, | |||||
.get_console_evtchn = hvm_get_console_evtchn, | |||||
.get_start_flags = hvm_get_start_flags, | |||||
}; |
Why is the const required, or useful, at all?