Hello Zhang,
I understand this is already applied, but I have some minor comments
for the future.
Aron
Zhang, Xing Z wrote: [Thu Jan 18 2007, 09:05:21PM EST]
Content-Description: donnot_panic_dom0_2.patch
> When there is not enough memory to create a domain,
> we not need panic domain0. Just prevent it from crating.
>
> Signed-off-by, Zhang Xin < xing.z.zhang@xxxxxxxxx >
> diff -r 58637a0a7c7e xen/arch/ia64/vmx/vmmu.c
> --- a/xen/arch/ia64/vmx/vmmu.c Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/vmx/vmmu.c Fri Jan 19 01:38:42 2007 +0800
> @@ -129,13 +129,16 @@ purge_machine_tc_by_domid(domid_t domid)
> #endif
> }
>
> -static void init_domain_vhpt(struct vcpu *v)
> +static int init_domain_vhpt(struct vcpu *v)
> {
> struct page_info *page;
> void * vbase;
> page = alloc_domheap_pages (NULL, VCPU_VHPT_ORDER, 0);
> if ( page == NULL ) {
> - panic_domain(vcpu_regs(v),"No enough contiguous memory for
> init_domain_vhpt\n");
> + //panic_domain(vcpu_regs(v),"No enough contiguous memory for
> init_domain_vhpt\n");
> + printk("No enough contiguous memory for init_domain_vhpt\n");
There's no need to comment out the panic_domain line. That's what
source control is for! :-) Better to just remove it.
> +
> + return -1;
> }
> vbase = page_to_virt(page);
> memset(vbase, 0, VCPU_VHPT_SIZE);
> @@ -147,18 +150,38 @@ static void init_domain_vhpt(struct vcpu
> VHPT(v,cch_sz) = VCPU_VHPT_SIZE - VHPT(v,hash_sz);
> thash_init(&(v->arch.vhpt),VCPU_VHPT_SHIFT-1);
> v->arch.arch_vmx.mpta = v->arch.vhpt.pta.val;
> -}
> -
> -
> -
> -void init_domain_tlb(struct vcpu *v)
> +
> + return 0;
> +}
> +
> +
> +static void free_domain_vhpt(struct vcpu *v)
> +{
> + struct page_info *page;
> +
> + if ( v->arch.vhpt.hash) {
> + page = virt_to_page(v->arch.vhpt.hash);
> + free_domheap_pages(page, VCPU_VHPT_ORDER);
> + }
> +
> + return;
> +}
> +
> +int init_domain_tlb(struct vcpu *v)
> {
> struct page_info *page;
> void * vbase;
> - init_domain_vhpt(v);
> +
> + if ( init_domain_vhpt(v) != 0 )
> + goto err;
> +
> page = alloc_domheap_pages (NULL, VCPU_VTLB_ORDER, 0);
> if ( page == NULL ) {
> - panic_domain(vcpu_regs(v),"No enough contiguous memory for
> init_domain_tlb\n");
> + //panic_domain("No enough contiguous memory for init_domain_tlb\n");
> + printk("No enough contiguous memory for init_domain_tlb\n");
Same here.
> + free_domain_vhpt(v);
> +
> + goto err;
> }
> vbase = page_to_virt(page);
> memset(vbase, 0, VCPU_VTLB_SIZE);
> @@ -169,7 +192,12 @@ void init_domain_tlb(struct vcpu *v)
> VTLB(v,cch_buf) = (void *)((u64)vbase + VTLB(v,hash_sz));
> VTLB(v,cch_sz) = VCPU_VTLB_SIZE - VTLB(v,hash_sz);
> thash_init(&(v->arch.vtlb),VCPU_VTLB_SHIFT-1);
> -}
> +
> + return 0;
> +err:
> + return -1;
> +}
If there's nothing to clean up, IMHO there is no need for goto err.
Instead you could just return the error code directly.
Is there a better error code to use in these cases than -1, for
example -ENOMEM?
> +
>
> void free_domain_tlb(struct vcpu *v)
> {
> @@ -179,10 +207,8 @@ void free_domain_tlb(struct vcpu *v)
> page = virt_to_page(v->arch.vtlb.hash);
> free_domheap_pages(page, VCPU_VTLB_ORDER);
> }
> - if ( v->arch.vhpt.hash) {
> - page = virt_to_page(v->arch.vhpt.hash);
> - free_domheap_pages(page, VCPU_VHPT_ORDER);
> - }
> +
> + free_domain_vhpt(v);
> }
>
> /*
> diff -r 58637a0a7c7e xen/arch/ia64/vmx/vmx_init.c
> --- a/xen/arch/ia64/vmx/vmx_init.c Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/vmx/vmx_init.c Thu Jan 18 09:56:06 2007 +0800
> @@ -290,7 +290,7 @@ static void vmx_release_assist_channel(s
> * Initialize VMX envirenment for guest. Only the 1st vp/vcpu
> * is registered here.
> */
> -void
> +int
> vmx_final_setup_guest(struct vcpu *v)
> {
> vpd_t *vpd;
> @@ -305,7 +305,8 @@ vmx_final_setup_guest(struct vcpu *v)
> * to this solution. Maybe it can be deferred until we know created
> * one as vmx domain */
> #ifndef HASH_VHPT
> - init_domain_tlb(v);
> + if ( init_domain_tlb(v) != 0 )
> + goto err;
> #endif
> vmx_create_event_channels(v);
>
> @@ -322,6 +323,10 @@ vmx_final_setup_guest(struct vcpu *v)
>
> /* Set up guest 's indicator for VTi domain*/
> set_bit(ARCH_VMX_DOMAIN, &v->arch.arch_vmx.flags);
> +
> + return 0;
> +err:
> + return -1;
> }
Same here.
> void
> diff -r 58637a0a7c7e xen/arch/ia64/xen/domain.c
> --- a/xen/arch/ia64/xen/domain.c Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/arch/ia64/xen/domain.c Thu Jan 18 09:56:06 2007 +0800
> @@ -585,8 +585,11 @@ int arch_set_info_guest(struct vcpu *v,
> if (test_bit(_VCPUF_initialised, &v->vcpu_flags))
> return 0;
>
> - if (d->arch.is_vti)
> - vmx_final_setup_guest(v);
> + if (d->arch.is_vti){
> + rc = vmx_final_setup_guest(v);
> + if ( rc != 0 )
> + return rc;
> + }
> else {
> rc = vcpu_late_initialise(v);
> if (rc != 0)
> diff -r 58637a0a7c7e xen/include/asm-ia64/vmmu.h
> --- a/xen/include/asm-ia64/vmmu.h Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/include/asm-ia64/vmmu.h Thu Jan 18 09:56:06 2007 +0800
> @@ -295,7 +295,7 @@ extern void purge_machine_tc_by_domid(do
> extern void purge_machine_tc_by_domid(domid_t domid);
> extern void machine_tlb_insert(struct vcpu *d, thash_data_t *tlb);
> extern ia64_rr vmmu_get_rr(struct vcpu *vcpu, u64 va);
> -extern void init_domain_tlb(struct vcpu *d);
> +extern int init_domain_tlb(struct vcpu *d);
> extern void free_domain_tlb(struct vcpu *v);
> extern thash_data_t * vsa_thash(PTA vpta, u64 va, u64 vrr, u64 *tag);
> extern thash_data_t * vhpt_lookup(u64 va);
> diff -r 58637a0a7c7e xen/include/asm-ia64/vmx.h
> --- a/xen/include/asm-ia64/vmx.h Wed Jan 17 21:45:34 2007 -0700
> +++ b/xen/include/asm-ia64/vmx.h Thu Jan 18 09:56:06 2007 +0800
> @@ -31,7 +31,7 @@ extern void identify_vmx_feature(void);
> extern void identify_vmx_feature(void);
> extern unsigned int vmx_enabled;
> extern void vmx_init_env(void);
> -extern void vmx_final_setup_guest(struct vcpu *v);
> +extern int vmx_final_setup_guest(struct vcpu *v);
> extern void vmx_save_state(struct vcpu *v);
> extern void vmx_load_state(struct vcpu *v);
> extern void vmx_setup_platform(struct domain *d);
> _______________________________________________
> Xen-ia64-devel mailing list
> Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-ia64-devel
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|